On Wed, 14 Oct 2020 04:27:34 GMT, Kim Barrett <kbarr...@openjdk.org> wrote:
>> Finally returning to this review that was started in April 2020. I've >> recast it as a github PR. I think the security concern raised by Gil >> has been adequately answered. >> https://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2020-April/029203.html >> https://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2020-July/030401.html >> https://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2020-August/030677.html >> https://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2020-September/030793.html >> >> Please review a new function: java.lang.ref.Reference.refersTo. >> >> This function is needed to test the referent of a Reference object without >> artificially extending the lifetime of the referent object, as may happen >> when calling Reference.get. Some garbage collectors require extending the >> lifetime of a weak referent when accessed, in order to maintain collector >> invariants. Lifetime extension may occur with any collector when the >> Reference is a SoftReference, as calling get indicates recent access. This >> new function also allows testing the referent of a PhantomReference, which >> can't be accessed by calling get. >> >> The new function uses native methods whose implementations are in the VM so >> they can use the Access API. It is the intent that these methods will be >> intrinsified by optimizing compilers like C2 or graal, but that hasn't been >> implemented yet. Bear that in mind before rushing off to change existing >> uses of Reference.get. >> >> There are two native methods involved, one in Reference and an override in >> PhantomReference, both package private in java.lang.ref. The reason for this >> split is to simplify the intrinsification. This is a change from the version >> from April 2020; that version had a single native method in Reference, >> implemented using the ON_UNKNOWN_OOP_REF Access reference strength category. >> However, adding support for that category in the compilers adds significant >> implementation effort and complexity. Splitting avoids that complexity. >> >> Testing: >> mach5 tier1 >> Locally (linux-x64) verified the new test passes with various garbage >> collectors. > > Kim Barrett has updated the pull request incrementally with three additional > commits since the last revision: > > - simplify test > - cleanup nits from Mandy > - use Object instead of TestObject src/java.base/share/classes/java/lang/ref/Reference.java line 362: > 360: /* Implementation of refersTo() for non-phantom references. > 361: */ > 362: native boolean refersTo0(Object o); 1. Why is it named `refersTo0`? How about sth more descriptive, e.g.`refersToNative`? 2. Nit: including some text that this method is overridden in `PhantomReference.java` in the comments will be more informative and helpful; otherwise, one might be wondering how non-phantom and phantom refs are distinguished here. ------------- PR: https://git.openjdk.java.net/jdk/pull/498
