RFR(M) 8176100: [REDO][REDO] G1 Needs pre barrier on dereference of weak JNI handles

[Mikael Gerdin]

Hi Coleen,

On 2017-03-14 20:35, coleen.phillimore at oracle.com wrote:
> This looks good still.   For

Thanks.


> https://bugs.openjdk.java.net/browse/JDK-8176363
>
> why are the two locks "special"?  Is that because there is no leaf-2?
> Just curious.

Unclear.
I guess it's because they are guarding a very small region and someone 
wanted to make sure that no other locks are taken when they are held.

/Mikael

>
> Thanks,
> Coleen
>
> On 3/14/17 9:11 AM, Mikael Gerdin wrote:
>> Here we go again...
>> This time around the fix is exactly the same but since I changed the
>> lock ranks in 8176363 we shouldn't be hitting any lock ordering
>> assertions.
>>
>>
>> Hi all,
>> Please review this revised change to jweak to support G1.
>>
>> I've copied Kim's original description of the fix below for reference:
>>
>> >
>> > The problem being addressed is that, when using G1, dereferencing a
>> > jweak must ensure the obtained referent is ensured live, just as for
>> > other weak reference types.  This has always been a requirement, and
>> > failure to do so appears to be a since-day1 G1 bug.
>> >
>> > There are two categories of places that need to address this issue:
>> > JNIHandles::resolve and friends, and returning a jobject from native
>> > code to Java.  In both of these places the jobject whose contained oop
>> > is being extracted might really be a jweak.  jweak is
>> > representationally indistinguishable from jobject; jweak is just a
>> > typedef for jobject.  The documentation says a jweak can be used
>> > anywhere a jobject is permitted, making that type equivalence
>> > necessary for a C API.  (A C++ API might be able to have distinct
>> > types via inheritance, though would still need a method for
>> > distinguishing them at runtime.  But since JNI is a C API...).
>> >
>> > The basic approach being taken is to "tag" jweaks by setting the low
>> > order bit (recall that jweak == jobject == _jobject*), in order to
>> > distinguish them from non-jweak jobjects.  This tagging is only being
>> > done when the selected GC needs the distinction, e.g. G1. This gives
>> > us a representational difference between jobject and jweak on which to
>> > base the different behavior, hiding that distinction behind the
>> > existing API.
>> >
>> > JNIHandles::resolve and friends are changed to unconditionally strip
>> > off any potential weak tag when translating from jobject to to oop*.
>> > That's cheaper than testing for the conditional use of tagging and
>> > then stripping.  Also stripped when deleting JNI handles.
>> >
>> > TemplateInterpreterGenerator::generate_native_entry and
>> > SharedRuntime::generate_native_wrapper are changed to conditionally
>> > emit code to apply the G1 pre-barrier to the value obtained from a
>> > jobject result, when the result is tagged as a jweak, which only
>> > occurs when using G1.
>> >
>> > For arm32/arm64, this required moving the g1_write_barrier_pre
>> > definitions from InterpreterMacroAssembler to MacroAssembler.  Also
>> > moved g1_write_barrier_post, for consistency.
>> >
>>
>> In addition to Kim's final version of the change (which was backed
>> out) this updated version adds code to mask out the weak tag bit in
>> the fast JNI field getters on all platforms where fast JNI getters are
>> used.
>>
>> Since the fast JNI getters only read primitive values from objects me
>> and Kim decided that there was no need to invoke the G1 pre-barrier if
>> a jweak was used as the "receiver" for the getters, this simplifies
>> the change to the fast getters and I was able to come up with a
>> seemingly efficient instruction encoding on all platforms.
>>
>> A couple of comments about the implementations:
>> My goal was to encode clearing of the weak_tag_mask (which at the
>> moment is 1).
>> On SPARC and 32 bit arm there is an instruction that can be used
>> straight off to perform an and with a value that is first negated,
>> ANDN on SPARC and BTC on ARM.
>> On 64 bit ARM the encoding for immediates allows encoding of the
>> bitwise inverse of 1 (0xfffffffffffffffe) but if weak_tag_mask is
>> changed the encoding may fail.
>> On x86 I pass the mask as a signed 32 bit immediate but note that the
>> assembler encodes it as an 8 bit immediate and sets a sign-extension
>> bit in the opcode.
>>
>>
>> Testing:
>> Standard JPRT and tier2-3 HotSpot tests.
>> I've modified the CallWithJNIWeak test to call all the primitive
>> getters and some other interesting cases I came up with.
>> I've also run the CallWithJNIWeak test on all platforms on both
>> product and fastdebug builds (since fast JNI getters are implicitly
>> disabled in debug builds due to VerifyJNIFields being enabled by
>> default in debug builds.
>>
>> The aarch64 change has been tested in the previous round of this change.
>>
>> Full webrev:
>> http://cr.openjdk.java.net/~mgerdin/8176100/webrev.0/
>>
>> Bugs:
>> https://bugs.openjdk.java.net/browse/JDK-8176100
>> https://bugs.openjdk.java.net/browse/JDK-8175085
>> https://bugs.openjdk.java.net/browse/JDK-8166188
>>
>> Thanks
>> /Mikael
>