RFR: 8247213: G1: Reduce usage of volatile in favour of Atomic operations
leo.korinth at oracle.com
Wed Jun 10 12:46:46 UTC 2020
Hi, could I have a review for this change that adds AtomicValue<> to
atomic.hpp and uses it in G1?
I am adding an AtomicValue<> to atomic.hpp. This is an opaque type (the
"value" is private) and that protect against non-atomic operations being
used by mistake. AtomicValue methods are proxied to the corresponding
(all-static) Atomic:: methods. All operations are explicitly atomic and
in a type safe manner with semantics defined in enum atomic_memory_order
(memory_order_conservative by default).
Instead of using field variables as volatile, I change them to be of
AtomicValue<> type. No need to verify that += (for example) will result
in an atomic instruction on all supported compilers.
I have some open questions regarding the exported fields in
vmStructs_g1.hpp. Today, volatile fields are sometimes "exported" as
volatile (and sometimes not, and I guess this is by mistake). I choose
to export them all as non-volatile. From what I can see the volatile
specific part only casts to void* (only documentation?). Java code is
unchanged and still access them as the unwrapped values (static assert
in atomic.hpp guarantees that memory layout is the same for T and
AtomicValue<T>). I think this is okay, but would like feedback on all this.
The change is presented as a two part change. The first part changes all
volatile to AtomicValue, the other part removes the AtomicValue part on
non-field accesses. By doing it two part I will not forget to transform
some operations by mistake.
Copyright years will be updated when all other changes are approved.
How about pushing this after 15 is branched off and thus have it for 16?
More information about the hotspot-gc-dev