RFR (M) 8249650: Optimize JNIHandle::make_local thread variable usage

Kim Barrett kim.barrett at oracle.com
Mon Jul 20 06:15:13 UTC 2020


> On Jul 20, 2020, at 1:53 AM, David Holmes <david.holmes at oracle.com> wrote:
> 
> Hi Kim,
> 
> Thanks for looking at this.
> 
> Updated webrev at:
> 
> http://cr.openjdk.java.net/~dholmes/8249650/webrev.v2/

Looks good.

> 
> On 20/07/2020 3:22 pm, Kim Barrett wrote:
>>> On Jul 20, 2020, at 12:16 AM, David Holmes <david.holmes at oracle.com> wrote:
>> src/hotspot/share/prims/jni.cpp
>>  743     result = JNIHandles::make_local(THREAD, result_handle());
>> jni_PopLocalFrame is now using a mix of "thread" and "THREAD", where
>> previously it just used "thread". Maybe this change shouldn't be made?
>> Or can the other uses be changed to THREAD for consistency?
> 
> "thread" and "THREAD" are interchangeable for anything expecting a "Thread*" (and somewhat surprisingly a number of API's that only work for JavaThreads actually take a Thread*. :( ). I had choice between trying to be file-wide consistent with the make_local calls, versus local-code consistent, and used THREAD as it is available in both JNI_ENTRY and via TRAPS. But I can certainly make a local change to "thread" for local consistency.

I don’t feel strongly either way.  It just struck me as a little odd to have the mix in close proximity,
especially since I think consistently using either one might work in this function.  But being consistent
about make_local usage has something to be said for it too.

>> src/hotspot/share/prims/jvm.cpp
>> The calls to JvmtiExport::post_vm_object_alloc have to use "thread"
>> instead of "THREAD", even though other places nearby are using
>> "THREAD".  That inconsistency is kind of unfortunate, but doesn't seem
>> easily avoidable.
> 
> Everything that uses THREAD in a JVM_ENTRY method can be changed to use "thread" instead. But I'm not sure it's a consistency worth pursuing at least as part of these changes (there are likely similar issues with most of the touched files).

Yeah, it’s not really obvious whether to use THREAD or thread in some cases.
But I agree that addressing any inconsistencies there is mostly out of scope for
this change.



More information about the serviceability-dev mailing list