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

Mikael Gerdin mikael.gerdin at oracle.com
Tue Mar 14 13:11:21 UTC 2017

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

More information about the hotspot-dev mailing list