[9] RFR(L) 8013267 : move MemberNameTable from native code to Java heap, use to intern MemberNames

[Peter Levart]

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 <david.holmes at 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();

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 
>>> core-libs-dev at openjdk.java.net
>>>
>>> David H.
>>>
>>> [1] 
>>> http://docs.oracle.com/javase/specs/jls/se8/html/jls-17.html#jls-17.4.4
>>>
>>>
>>>> David
>