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

[David Holmes]

Hi Kim,

On 22/07/2020 6:04 pm, Kim Barrett wrote:
>> 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.

Thanks for the follow up - I figured it was all "on hold".

Cheers,
David

> 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.
>