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

Kim Barrett kim.barrett at oracle.com
Thu Apr 9 04:00:56 UTC 2020


> On Apr 8, 2020, at 5:44 AM, Aleksey Shipilev <shade at redhat.com> wrote:
> 
> On 4/8/20 2:25 AM, Kim Barrett wrote:
>> Webrev:
>> https://cr.openjdk.java.net/~kbarrett/8188055/open.04/
> 
> src/hotspot/share/prims/jvm.cpp:
> 
> *) Do we really need a typedef (L3250) for something that is used once at L3253?
> 
> 3248 JVM_ENTRY(jboolean, JVM_ReferenceRefersTo(JNIEnv* env, jobject ref, jobject o))
> 3249   JVMWrapper("JVM_ReferenceRefersTo");
> 3250   typedef HeapAccess<ON_UNKNOWN_OOP_REF | AS_NO_KEEPALIVE> ReferentAccess;
> 3251   const int referent_offset = java_lang_ref_Reference::referent_offset;
> 3252   oop ref_oop = JNIHandles::resolve_non_null(ref);
> 3253   oop referent = ReferentAccess::oop_load_at(ref_oop, referent_offset);
> 3254   return referent == JNIHandles::resolve(o);
> 3255 JVM_END

Given a choice between a typedef, an excessively long line (especially
without a similarly used-once local constant for referent_offset), or
an awkwardly placed line-break, I prefer the first.

> *) Double new-line:
> 
> 3256
> 3257

Double new-line separations seem to be the norm in this file. It seems
that didn't get followed for the between JVM_ENTRY blocks in this
section though. (I forget whether it was me or Per who wrote those,
but I did the push, so mea culpa.)

> test/hotspot/jtreg/gc/TestReferenceRefersTo.java:
> 
> *) Typo, "unex[p]ected":
> 
> 134                 fail(which + " refers to unexected value”);

Fixed.

> Otherwise seems fine.

Thanks.

Waiting for other discussion to resolve before sending out any new webrevs.



More information about the core-libs-dev mailing list