RFR: 8188055: (ref) Add Reference.refersTo predicate

[Kim Barrett]

> On Apr 8, 2020, at 5:27 AM, David Holmes <david.holmes at oracle.com> wrote:
> 
> Hi Kim,

Apparently I lost track of these comments and forgot to respond.

I still won't be sending out a new webrev until some of the other
discussion gets settled.  There's been some internal discussion, but
I'm currently waiting on some other folks to have time to chime in.

Replies to your comments inline below.

> On 8/04/2020 10:25 am, Kim Barrett wrote:
>> [Note review on both core-libs and hotspot-gc-dev lists; try not to lose
>> either when replying.]
>> 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.
> 
> This causes a slight conflict with the documentation for get() which states:
> 
> "Because the referent of a phantom reference is always inaccessible ..."
> 
> when the new method obviously accesses it.

I take that "inaccessible" to mean "inaccessible to the application",
and refersTo doesn't make the referent accessible to the application.

>> The new function uses a native method whose implementation is in the
>> VM so it can use the Access API.  It is the intent that this function
>> 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.
> 
> I assume such intrinsification is intended for JDK 15 as well though, as end users may well rush to change their code!

Looks like we missed the JDK 15 train.

>> CR:
>> https://bugs.openjdk.java.net/browse/JDK-8188055
>> https://bugs.openjdk.java.net/browse/JDK-8241029 (CSR)
> 
> In the specification:
> 
> * @param   obj is the object to compare with the referenced object, or
> *          {@code null}.
> 
> First delete "is", the commone style for @param is just to say "the xxx" or "a yyy”.

Done locally.

> Second I suggest:
> 
> s/the referenced object/this reference object's referent/

Done locally.

> In the apinote:
> 
> * collection cycle.  {@link #refersTo(Object) refersTo} can be used to
> 
> I suggest:
> 
> * collection cycle.  The {@link #refersTo(Object) refersTo} method can be used to
> 
> so we don't start a sentence with a lower-case code-font word.

Done locally.

> Also a query in the apinote:
> 
> * This method returns a strong reference to the referent. This may cause
> * the garbage collector to treat it as strongly reachable until some later
> 
> Surely if the method returns a strong reference then the GC _will_ treat it as strongly reachable, not "may” ?

Something like refersTo is needed because an access using get may
force some phases of some collectors to treat the referent as strongly
reachable for some additional period, even if the application
immediately drops all references to it.  Other collectors may not need
to do anything of the sort.  And even collectors that do sometimes
need to do so may not always need to do so.  It's all vaguely
weasel-worded because there is so much potential variation.

>> Webrev:
>> https://cr.openjdk.java.net/~kbarrett/8188055/open.04/
> 
> Code changes look good. No comment on the test - I'll leave it to others to analyse that.

Thanks.