From 618358bd2dedebb1bd8f41b1382d2bc028f1865d Mon Sep 17 00:00:00 2001 From: Babneet Singh Date: Wed, 8 Nov 2023 19:20:13 -0500 Subject: [PATCH 1/3] Remove unused variables in jvm.c This improves perf by saving the resources used to store and initialize these variable. Signed-off-by: Babneet Singh --- runtime/j9vm/javanextvmi.cpp | 6 ------ runtime/j9vm/jvm.c | 26 +++++--------------------- 2 files changed, 5 insertions(+), 27 deletions(-) diff --git a/runtime/j9vm/javanextvmi.cpp b/runtime/j9vm/javanextvmi.cpp index 3386dd61d6b..97f9fe4c450 100644 --- a/runtime/j9vm/javanextvmi.cpp +++ b/runtime/j9vm/javanextvmi.cpp @@ -40,12 +40,6 @@ extern "C" { #if JAVA_SPEC_VERSION >= 19 extern J9JavaVM *BFUjavaVM; - -/* These come from jvm.c */ -extern IDATA (*f_monitorEnter)(omrthread_monitor_t monitor); -extern IDATA (*f_monitorExit)(omrthread_monitor_t monitor); -extern IDATA (*f_monitorWait)(omrthread_monitor_t monitor); -extern IDATA (*f_monitorNotifyAll)(omrthread_monitor_t monitor); #endif /* JAVA_SPEC_VERSION >= 19 */ /* Define for debug diff --git a/runtime/j9vm/jvm.c b/runtime/j9vm/jvm.c index d702239b128..2af20c3f06e 100644 --- a/runtime/j9vm/jvm.c +++ b/runtime/j9vm/jvm.c @@ -209,11 +209,7 @@ typedef IDATA (*MonitorEnter)(omrthread_monitor_t monitor); typedef IDATA (*MonitorExit)(omrthread_monitor_t monitor); typedef IDATA (*MonitorInit)(omrthread_monitor_t* handle, UDATA flags, char* name); typedef IDATA (*MonitorDestroy)(omrthread_monitor_t monitor); -typedef IDATA (*MonitorWait)(omrthread_monitor_t monitor); -typedef IDATA (*MonitorWaitTimed)(omrthread_monitor_t monitor, I_64 millis, IDATA nanos); -typedef IDATA (*MonitorNotify)(omrthread_monitor_t monitor); -typedef IDATA (*MonitorNotifyAll)(omrthread_monitor_t monitor) ; -typedef IDATA (* ThreadLibControl)(const char * key, UDATA value) ; +typedef IDATA (* ThreadLibControl)(const char * key, UDATA value); typedef IDATA (*SetCategory)(omrthread_t thread, UDATA category, UDATA type); typedef IDATA (*LibEnableCPUMonitor)(omrthread_t thread); @@ -270,12 +266,8 @@ static ThreadDetach f_threadDetach; /* Share these with the other *.c in the DLL */ MonitorEnter f_monitorEnter; MonitorExit f_monitorExit; -MonitorWait f_monitorWait; -MonitorNotifyAll f_monitorNotifyAll; static MonitorInit f_monitorInit; static MonitorDestroy f_monitorDestroy; -static MonitorWaitTimed f_monitorWaitTimed; -static MonitorNotify f_monitorNotify; static PortInitLibrary portInitLibrary; static PortGetSize portGetSizeFn; static PortGetVersion portGetVersionFn; @@ -1020,15 +1012,11 @@ preloadLibraries(void) f_monitorExit = (MonitorExit) GetProcAddress (threadDLL, (LPCSTR) "omrthread_monitor_exit"); f_monitorInit = (MonitorInit) GetProcAddress (threadDLL, (LPCSTR) "omrthread_monitor_init_with_name"); f_monitorDestroy = (MonitorDestroy) GetProcAddress (threadDLL, (LPCSTR) "omrthread_monitor_destroy"); - f_monitorWait = (MonitorWait) GetProcAddress (threadDLL, (LPCSTR) "omrthread_monitor_wait"); - f_monitorWaitTimed = (MonitorWaitTimed) GetProcAddress (threadDLL, (LPCSTR) "omrthread_monitor_wait_timed"); - f_monitorNotify = (MonitorNotify) GetProcAddress (threadDLL, (LPCSTR) "omrthread_monitor_notify"); - f_monitorNotifyAll = (MonitorNotifyAll) GetProcAddress (threadDLL, (LPCSTR) "omrthread_monitor_notify_all"); f_threadLibControl = (ThreadLibControl) GetProcAddress (threadDLL, (LPCSTR) "omrthread_lib_control"); f_setCategory = (SetCategory) GetProcAddress (threadDLL, (LPCSTR) "omrthread_set_category"); f_libEnableCPUMonitor = (LibEnableCPUMonitor) GetProcAddress (threadDLL, (LPCSTR) "omrthread_lib_enable_cpu_monitor"); - if (!f_threadGlobal || !f_threadAttachEx || !f_threadDetach || !f_monitorEnter || !f_monitorExit || !f_monitorInit || !f_monitorDestroy || !f_monitorWaitTimed - || !f_monitorNotify || !f_monitorNotifyAll || !f_threadLibControl || !f_setCategory || !f_libEnableCPUMonitor || !f_monitorWait + if (!f_threadGlobal || !f_threadAttachEx || !f_threadDetach || !f_monitorEnter || !f_monitorExit || !f_monitorInit + || !f_monitorDestroy || !f_threadLibControl || !f_setCategory || !f_libEnableCPUMonitor ) { FreeLibrary(vmDLL); FreeLibrary(threadDLL); @@ -1451,15 +1439,11 @@ preloadLibraries(void) f_monitorExit = (MonitorExit) dlsym (threadDLL, "omrthread_monitor_exit"); f_monitorInit = (MonitorInit) dlsym (threadDLL, "omrthread_monitor_init_with_name"); f_monitorDestroy = (MonitorDestroy) dlsym (threadDLL, "omrthread_monitor_destroy"); - f_monitorWait = (MonitorWait) dlsym (threadDLL, "omrthread_monitor_wait"); - f_monitorWaitTimed = (MonitorWaitTimed) dlsym (threadDLL, "omrthread_monitor_wait_timed"); - f_monitorNotify = (MonitorNotify) dlsym (threadDLL, "omrthread_monitor_notify"); - f_monitorNotifyAll = (MonitorNotifyAll) dlsym (threadDLL, "omrthread_monitor_notify_all"); f_threadLibControl = (ThreadLibControl) dlsym (threadDLL, "omrthread_lib_control"); f_setCategory = (SetCategory) dlsym (threadDLL, "omrthread_set_category"); f_libEnableCPUMonitor = (LibEnableCPUMonitor) dlsym (threadDLL, "omrthread_lib_enable_cpu_monitor"); - if (!f_threadGlobal || !f_threadAttachEx || !f_threadDetach || !f_monitorEnter || !f_monitorExit || !f_monitorInit || !f_monitorDestroy || !f_monitorWaitTimed - || !f_monitorNotify || !f_monitorNotifyAll || !f_threadLibControl || !f_setCategory || !f_libEnableCPUMonitor || !f_monitorWait + if (!f_threadGlobal || !f_threadAttachEx || !f_threadDetach || !f_monitorEnter || !f_monitorExit || !f_monitorInit + || !f_monitorDestroy || !f_threadLibControl || !f_setCategory || !f_libEnableCPUMonitor ) { dlclose(vmDLL); #ifdef J9ZOS390 From 661d3994218e325256b8c53acb6938b065742b45 Mon Sep 17 00:00:00 2001 From: Babneet Singh Date: Wed, 8 Nov 2023 19:50:57 -0500 Subject: [PATCH 2/3] Update error handling in JVMTI NotifyFramePop Return JVMTI_ERROR_THREAD_NOT_SUSPENDED if the thread was not suspended and was not the current thread. For virtual threads, VirtualThread.state is read and it is checked if JVMTI_VTHREAD_STATE_SUSPENDED is set in VirtualThread.state. This check is incorrect because JVMTI_VTHREAD_STATE_SUSPENDED represents an internal state at the Java level. It does not imply if the virtual thread is suspended through JVMTI. JVMTI NotifyFramePop is updated to check if the virtual thread is suspended through JVMTI. Signed-off-by: Babneet Singh --- runtime/jvmti/jvmtiStackFrame.c | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/runtime/jvmti/jvmtiStackFrame.c b/runtime/jvmti/jvmtiStackFrame.c index f4130beb014..026425a4579 100644 --- a/runtime/jvmti/jvmtiStackFrame.c +++ b/runtime/jvmti/jvmtiStackFrame.c @@ -617,29 +617,25 @@ jvmtiNotifyFramePop(jvmtiEnv *env, J9JVMTI_GETVMTHREAD_ERROR_ON_DEAD_THREAD); if (JVMTI_ERROR_NONE == rc) { #if JAVA_SPEC_VERSION >= 19 - BOOLEAN isVThreadSuspended = FALSE; + j9object_t threadObject = (NULL == thread) ? currentThread->threadObject : J9_JNI_UNWRAP_REFERENCE(thread); if (NULL != targetThread) #endif /* JAVA_SPEC_VERSION >= 19 */ { vm->internalVMFunctions->haltThreadForInspection(currentThread, targetThread); } -#if JAVA_SPEC_VERSION >= 19 - if ((NULL != thread) && (NULL == targetThread)) { - /* The assert in getVMThread will assure that this is a virtual thread */ - jint vthreadState = J9VMJAVALANGVIRTUALTHREAD_STATE(currentThread, J9_JNI_UNWRAP_REFERENCE(thread)); - isVThreadSuspended = OMR_ARE_ANY_BITS_SET(vthreadState, JVMTI_VTHREAD_STATE_SUSPENDED); - } -#endif /* JAVA_SPEC_VERSION >= 19 */ - if ((currentThread == targetThread) + /* Error if the thread is not suspended and not the current thread. */ + if ((currentThread != targetThread) #if JAVA_SPEC_VERSION >= 19 - || isVThreadSuspended + && (0 == J9OBJECT_U32_LOAD(currentThread, threadObject, vm->isSuspendedInternalOffset)) +#else /* JAVA_SPEC_VERSION >= 19 */ + && OMR_ARE_NO_BITS_SET(targetThread->publicFlags, J9_PUBLIC_FLAGS_HALT_THREAD_JAVA_SUSPEND) #endif /* JAVA_SPEC_VERSION >= 19 */ - || ((NULL != targetThread) && OMR_ARE_ANY_BITS_SET(targetThread->publicFlags, J9_PUBLIC_FLAGS_HALT_THREAD_JAVA_SUSPEND)) - ) { + ) { + rc = JVMTI_ERROR_THREAD_NOT_SUSPENDED; + } else { J9StackWalkState walkState = {0}; J9VMThread *threadToWalk = targetThread; - #if JAVA_SPEC_VERSION >= 19 J9VMThread stackThread = {0}; J9VMEntryLocalStorage els = {0}; @@ -669,9 +665,8 @@ jvmtiNotifyFramePop(jvmtiEnv *env, } } } - } else { - rc = JVMTI_ERROR_THREAD_NOT_SUSPENDED; } + #if JAVA_SPEC_VERSION >= 19 if (NULL != targetThread) #endif /* JAVA_SPEC_VERSION >= 19 */ From 7c82a956b043f61069f5f02021235a2405e603ab Mon Sep 17 00:00:00 2001 From: Babneet Singh Date: Wed, 8 Nov 2023 20:43:38 -0500 Subject: [PATCH 3/3] Update acquireVThreadInspector and enterVThreadTransitionCritical VM is entered by the invocation of internalEnterVMFromJNI. VM is exited by the invocation of internalExitVMToJNI. VM is always entered before acquireVThreadInspector and enterVThreadTransitionCritical are invoked. In these functions, the goal can be achieved by just releasing and re-acquiring VM access. The new changes just release and re-acquire VM access instead of exiting and entering the VM. This has less overhead. Addresses: https://github.com/eclipse-openj9/openj9/pull/18420#issuecomment-1802476741 Signed-off-by: Babneet Singh --- runtime/j9vm/javanextvmi.cpp | 4 ++-- runtime/vm/ContinuationHelpers.cpp | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/runtime/j9vm/javanextvmi.cpp b/runtime/j9vm/javanextvmi.cpp index 97f9fe4c450..a50c0b07665 100644 --- a/runtime/j9vm/javanextvmi.cpp +++ b/runtime/j9vm/javanextvmi.cpp @@ -274,10 +274,10 @@ enterVThreadTransitionCritical(J9VMThread *currentThread, jobject thread) while(!objectAccessBarrier.inlineMixedObjectCompareAndSwapU64(currentThread, threadObj, vm->virtualThreadInspectorCountOffset, 0, (U_64)-1)) { /* Thread is being inspected or unmounted, wait. */ - vmFuncs->internalExitVMToJNI(currentThread); + vmFuncs->internalReleaseVMAccess(currentThread); VM_AtomicSupport::yieldCPU(); /* After wait, the thread may suspend here. */ - vmFuncs->internalEnterVMFromJNI(currentThread); + vmFuncs->internalAcquireVMAccess(currentThread); threadObj = J9_JNI_UNWRAP_REFERENCE(thread); } } diff --git a/runtime/vm/ContinuationHelpers.cpp b/runtime/vm/ContinuationHelpers.cpp index e88f7e7652c..2b5931b4fda 100644 --- a/runtime/vm/ContinuationHelpers.cpp +++ b/runtime/vm/ContinuationHelpers.cpp @@ -539,10 +539,10 @@ acquireVThreadInspector(J9VMThread *currentThread, jobject thread, BOOLEAN spin) vthreadInspectorCount = J9OBJECT_I64_LOAD(currentThread, threadObj, vm->virtualThreadInspectorCountOffset); if (vthreadInspectorCount < 0) { /* Thread is in transition, wait. */ - vmFuncs->internalExitVMToJNI(currentThread); + vmFuncs->internalReleaseVMAccess(currentThread); VM_AtomicSupport::yieldCPU(); /* After wait, the thread may suspend here. */ - vmFuncs->internalEnterVMFromJNI(currentThread); + vmFuncs->internalAcquireVMAccess(currentThread); if (spin) { goto retry; } else {