> On Jul 20, 2020, at 12:16 AM, David Holmes <david.hol...@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. ------------------------------------------------------------------------------
