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