RFR: 8188055: (ref) Add Reference::refersTo predicate
Kim Barrett
kim.barrett at oracle.com
Thu Oct 8 07:52:50 UTC 2020
> On Oct 5, 2020, at 7:04 AM, Per Liden <pliden at openjdk.java.net> wrote:
> test/hotspot/jtreg/gc/TestReferenceRefersTo.java line 179:
>
>> 177: // For some collectors, calling get() will keep testObject4 alive.
>> 178: // For example, SATB collectors or ZGC, but not collectors using
>> 179: // incremental update barriers like CMS.
>
> Remove obsolete CMS comment?
I'll just delete lines 178-9.
> test/hotspot/jtreg/gc/TestReferenceRefersTo.java line 65:
>
>> 63:
>> 64: private static WeakReference<TestObject> testWeak2 = null;
>> 65: private static WeakReference<TestObject> testWeak3 = null;
>
> It looks like test*2 and test*3 test the same thing in both tests, so test*3 could be removed, and test*4 could be
> renamed test*3.
The test seems to be missing some stuff. This looks like the test from the
webrev back in April too. I looked through some backups and never published
webrevs and found some other versions, but nothing that looks like the final
version I remember. Oh well. So I'm adding some additional checks that make
the 2 & 3 cases differ as intended.
> src/hotspot/share/prims/jvm.cpp line 3446:
>
>> 3444: oop referent = HeapAccess<on_ref | AS_NO_KEEPALIVE>::oop_load_at(ref_oop, offset);
>> 3445: return referent == JNIHandles::resolve(o);
>> 3446: }
>
> How about moving the inner logic to java_lang_ref_{Reference,PhantomReference}::refers_to() functions?
I think refersTo functionality doesn't really belong in
java_lang_ref_Reference. I would rather keep the refersTo implementation in
jvm.cpp, but change the implementation from bare Access to using something
like java_lang_ref_Reference::referent.
But it seems referent() does something rather wrong. It's doing a strong
access, and that just happens to "work" because the places calling it are
code paths that are only used when a strong access doesn't do anything
special. I think those places ought to instead be using ON_UNKNOWN_OOP_REF
and AS_NO_KEEPALIVE. I'm making some changes there and changing jvm.cpp
accordingly.
More information about the core-libs-dev
mailing list