> On Apr 8, 2020, at 3:32 AM, Per Liden <per.li...@oracle.com> wrote:
> On 4/8/20 2:25 AM, Kim Barrett wrote:
>> 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.
> 
> Do we have an enhancement filed for this?

Not yet.

> We should make it very clear that such an intrinsic is not allowed to 
> safepoint between loading the referent and comparing it.
> 
>> CR:
>> https://bugs.openjdk.java.net/browse/JDK-8188055
>> https://bugs.openjdk.java.net/browse/JDK-8241029 (CSR)
>> Webrev:
>> https://cr.openjdk.java.net/~kbarrett/8188055/open.04/
> 
> The patch looks good to me, just two minor comments:
> 
> 
> java/lang/ref/Reference.java
> ----------------------------
> 
> 349      * @param   obj is the object to compare with the referenced object, 
> or
> 350      *          {@code null}.
> 
> May I suggest "referenced object" -> "referent" to keep the nomenclature 
> intact.

That seems reasonable, but I’m going to wait for some of the other discussion to
resolve before making any changes to the Javadoc.

> gc/TestReferenceRefersTo.java
> -----------------------------
> 
> 208             long timeout = 1000;
> 209             while (true) {
> 210                 Reference<? extends TestObject> ref = 
> queue.remove(timeout);
> 211                 if (ref == null) {
> 212                     break;
> 213                 } else if (ref == testPhantom1) {
> 214                     testPhantom1 = null;
> 215                 } else if (ref == testWeak2) {
> 216                     testWeak2 = null;
> 217                 } else if (ref == testWeak3) {
> 218                     testWeak3 = null;
> 219                 } else if (ref == testWeak4) {
> 220                     testWeak4 = null;
> 221                 } else {
> 222                     fail("unexpected reference in queue");
> 223                 }
> 224             }
> 
> That timeout look unnecessarily short, and I imagine it could cause 
> intermittent failures on really slow machines. I would suggest we make that 
> more like 60 seconds, alternatively skip the timeout all together and let the 
> whole test timeout if the expected references don't appear in the queue.

Increasing the timeout seems reasonable.  I’m not so sure about removing the 
limit completely
though.  I think we don’t always get much state information on a test timeout, 
which would
make it harder to distinguish this specific failure from (say) a *really* slow 
machine taking a
long time to execute the test.

Reply via email to