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