RFR(M): 8033552: Fix missing missing volatile specifiers in CAS operations in GC code
Erik Österlund
erik.osterlund at oracle.com
Wed Apr 6 18:54:23 UTC 2016
Hi,
Bug: https://bugs.openjdk.java.net/browse/JDK-8033552
CR: http://cr.openjdk.java.net/~eosterlund/8033552/webrev.00/
Basically, there have been issues with Atomic::* operations being used
on fields declared non-volatile. Especially in loop load phi cas
phi-style code where the reload of a non-volatile field is not done
because the compiler is allowed to assume it has not changed and keep
the value in register.
My proposed patch fixes this by declaring all fields used in GC code
volatile. As David Holmes said - all such code should use volatile
unless there are peculiar reasons for specific code not to for
performance reasons.
I also changed the cas phi load phi loops to not reload the field, for
the simple reason that it is unnecessary - the CAS already performs the
required load, so there is no need to do it again. So whether safe or
not do do this with volatiles, a pointless load is a pointless load, so
I removed them. (cf. g1ConcurrentMark.cpp and workgroup.cpp)
Testing:
* JPRT
* Benchmarked with SPECjbb2005 and SPECjvm2008 to make sure there was no
regression
I will need a sponsor to push this.
Thanks,
/Erik
More information about the hotspot-gc-dev
mailing list