On 11/03/2014 05:16 PM, Peter Levart wrote:
On 11/03/2014 01:49 PM, David Chase wrote:
On 2014-11-02, at 10:49 PM, David Holmes <[email protected]>
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();
You might need a storeFence() between each two writes into the array
too. Your moving loop is the following:
2544 for (int i = oldCapacity; i > index; i--) {
2545 // pre: element_data[i] is duplicated at [i+1]
2546 element_data[i] = element_data[i - 1];
2547 // post: element_data[i-1] is duplicated at [i]
2548 }
If we start unrolling, it becomes:
w1: element_data[old_capacity - 0] = element_data[old_capacity - 1];
w2: element_data[old_capacity - 1] = element_data[old_capacity - 2];
w3: element_data[old_capacity - 2] = element_data[old_capacity - 3];
...
Can compiler reorder w2 and w3 (just writes - not the whole statements)?
Say that it reads a chunk of elements into the registers and then writes
them out, but in different order, and a check for safepoint comes inside
this chunk of writes... This is hypothetical, but it could do it without
breaking the local semantics...
Peter
Regards, Peter
BTW the Java side of this needs to be reviewed on
[email protected]
David H.
[1]
http://docs.oracle.com/javase/specs/jls/se8/html/jls-17.html#jls-17.4.4
David
