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
> 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