On 11/03/2014 01:49 PM, David Chase wrote:
On 2014-11-02, at 10:49 PM, David Holmes <david.hol...@oracle.com> wrote:
The change is to load the volatile size for the loop bound; this stops the
stores
in the loop from moving earlier, right?
Treating volatile accesses like memory barriers is playing a bit fast-and-loose with the
spec+implementation. The basic happens-before relationship for volatiles states that if a volatile
read sees a value X, then the volatile write that wrote X happened-before the read [1]. But in this
code there are no checks of the values of the volatile fields. Instead you are relying on a
volatile read "acting like acquire()" and a volatile write "acting like
release()".
That said you are trying to "synchronize" the hotspot code with the JDK code so
you have stepped outside the JMM in any case and reasoning about what is and is not
allowed is somewhat moot - unless the hotspot code always uses Java-style accesses to the
Java-level variables.
My main concern is that the compiler is inhibited from any peculiar code
motion; I assume that taking a safe point has a bit of barrier built into it
anyway, especially given that the worry case is safepoint + JVMTI.
Given the worry, what’s the best way to spell “barrier” here?
I could synchronize on classData (it would be a recursive lock in the current
version of the code)
synchronized (this) { size++; }
or I could synchronize on elementData (no longer used for a lock elsewhere, so
always uncontended)
synchronized (elementData) { size++; }
or is there some Unsafe thing that would be better?
(core-libs-dev — there will be another webrev coming. This is a runtime+jdk
patch.)
David
Hi David,
You're worried that writes moving array elements up for one slot would
bubble up before write of size = size+1, right? If that happens, VM
could skip an existing (last) element and not update it.
It seems that Unsafe.storeFence() between size++ and moving of elements
could do, as the javadoc for it says:
/**
* Ensures lack of reordering of stores before the fence
* with loads or stores after the fence.
* @since 1.8
*/
public native void storeFence();
Regards, Peter
BTW the Java side of this needs to be reviewed on core-libs-dev@openjdk.java.net
David H.
[1] http://docs.oracle.com/javase/specs/jls/se8/html/jls-17.html#jls-17.4.4
David
