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
>> -----
------------------------------------------------------------------------------
src/hotspot/share/classfile/javaClasses.cpp
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.
------------------------------------------------------------------------------
src/hotspot/share/classfile/verifier.cpp
299 JNIEnv *env = thread->jni_environment();
env now seems to only be used at line 320. Move this closer.
------------------------------------------------------------------------------
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?
------------------------------------------------------------------------------
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.
------------------------------------------------------------------------------
More information about the serviceability-dev
mailing list