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

Kim Barrett kim.barrett at oracle.com
Mon Jul 20 05:22:49 UTC 2020

> On Jul 20, 2020, at 12:16 AM, David Holmes <david.holmes at oracle.com> wrote:
> Subject line got truncated by accident ...
> On 20/07/2020 11:06 am, David Holmes wrote:
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8249650
>> webrev: http://cr.openjdk.java.net/~dholmes/8249650/webrev/
>> This is a simple cleanup that touches files across a number of VM areas - hence the cross-post.
>> Whilst working on a different JNI fix I noticed that in most cases in jni.cpp we were using the following form of make_local:
>> JNIHandles::make_local(env, obj);
>> and what that form does is first extract the thread from the JNIEnv:
>> JavaThread* thread = JavaThread::thread_from_jni_environment(env);
>> return thread->active_handles()->allocate_handle(obj);
>> but there is also another, faster, variant for when you already have the "thread":
>> jobject JNIHandles::make_local(Thread* thread, oop obj) {
>>   return thread->active_handles()->allocate_handle(obj);
>> }
>> When you look at the JNI_ENTRY wrapper (and related JVM_ENTRY, WB_ENTRY, UNSAFE_ENTRY etc) it has already extracted the thread from the JNIEnv:
>>     JavaThread* thread=JavaThread::thread_from_jni_environment(env);
>> and further defined:
>>     Thread* THREAD = thread;
>> so we always already have direct access to the "thread" available (or indirect via TRAPS), and in fact we can end up removing the make_local(JNIEnv* env, oop obj) variant altogether.
>> Along the way I spotted some related issues with unnecessary use of Thread::current() when it is already available from TRAPS, and some other cases where we extracted the JNIEnv from a thread only to later extract the thread from the JNIEnv.
>> Testing: tiers 1 - 3
>> Thanks,
>> David
>> -----

 439     JNIEnv *env = thread->jni_environment();

Since env is no longer used on the next line, move this down to where
it is used, at line 444.

 299   JNIEnv *env = thread->jni_environment();

env now seems to only be used at line 320.  Move this closer.

 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?


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.


More information about the hotspot-runtime-dev mailing list