From d926306594a99325791ba1a6d79ac8ae7671eb56 Mon Sep 17 00:00:00 2001 From: Sergey Nazarkin Date: Fri, 24 Jan 2025 12:41:31 +0200 Subject: [PATCH 1/3] Backport e704c055a4cf2aab77cc2b3d034f5a8b8d9e3331 --- src/hotspot/share/prims/jvm.cpp | 3 ++- src/hotspot/share/runtime/globals.hpp | 4 ++++ src/hotspot/share/runtime/mutexLocker.cpp | 3 +++ src/hotspot/share/runtime/mutexLocker.hpp | 2 ++ src/hotspot/share/runtime/threads.cpp | 6 ++++-- 5 files changed, 15 insertions(+), 3 deletions(-) diff --git a/src/hotspot/share/prims/jvm.cpp b/src/hotspot/share/prims/jvm.cpp index 7852f37d4be..958cbe72828 100644 --- a/src/hotspot/share/prims/jvm.cpp +++ b/src/hotspot/share/prims/jvm.cpp @@ -2955,9 +2955,10 @@ JVM_ENTRY(void, JVM_StartThread(JNIEnv* env, jobject jthread)) // We must release the Threads_lock before we can post a jvmti event // in Thread::start. { + ConditionalMutexLocker throttle_ml(ThreadsLockThrottle_lock, UseThreadsLockThrottleLock); // Ensure that the C++ Thread and OSThread structures aren't freed before // we operate. - MutexLocker mu(Threads_lock); + MutexLocker ml(Threads_lock); // Since JDK 5 the java.lang.Thread threadStatus is used to prevent // re-starting an already started thread, so we should usually find diff --git a/src/hotspot/share/runtime/globals.hpp b/src/hotspot/share/runtime/globals.hpp index 75d1eb1c89f..fab276220e0 100644 --- a/src/hotspot/share/runtime/globals.hpp +++ b/src/hotspot/share/runtime/globals.hpp @@ -1989,6 +1989,10 @@ const int ObjectAlignmentInBytes = 8; "more eagerly at the cost of higher overhead. A value of 0 " \ "(default) disables native heap trimming.") \ range(0, UINT_MAX) \ + \ + product(bool, UseThreadsLockThrottleLock, true, DIAGNOSTIC, \ + "Use an extra lock during Thread start and exit to alleviate" \ + "contention on Threads_lock.") \ // end of RUNTIME_FLAGS diff --git a/src/hotspot/share/runtime/mutexLocker.cpp b/src/hotspot/share/runtime/mutexLocker.cpp index 467fdb5a29a..a8ebba66022 100644 --- a/src/hotspot/share/runtime/mutexLocker.cpp +++ b/src/hotspot/share/runtime/mutexLocker.cpp @@ -66,6 +66,7 @@ Monitor* CodeCache_lock = nullptr; Mutex* TouchedMethodLog_lock = nullptr; Mutex* RetData_lock = nullptr; Monitor* VMOperation_lock = nullptr; +Monitor* ThreadsLockThrottle_lock = nullptr; Monitor* Threads_lock = nullptr; Mutex* NonJavaThreadsList_lock = nullptr; Mutex* NonJavaThreadsListSync_lock = nullptr; @@ -326,6 +327,8 @@ void mutex_init() { MUTEX_DEFN(JVMCIRuntime_lock , PaddedMonitor, safepoint, true); #endif + MUTEX_DEFN(ThreadsLockThrottle_lock , PaddedMonitor, safepoint); + // These locks have relative rankings, and inherit safepoint checking attributes from that rank. MUTEX_DEFL(InlineCacheBuffer_lock , PaddedMutex , CompiledIC_lock); MUTEX_DEFL(VtableStubs_lock , PaddedMutex , CompiledIC_lock); // Also holds DumpTimeTable_lock diff --git a/src/hotspot/share/runtime/mutexLocker.hpp b/src/hotspot/share/runtime/mutexLocker.hpp index cf280139dcc..6b241132246 100644 --- a/src/hotspot/share/runtime/mutexLocker.hpp +++ b/src/hotspot/share/runtime/mutexLocker.hpp @@ -61,6 +61,8 @@ extern Monitor* CodeCache_lock; // a lock on the CodeCache extern Mutex* TouchedMethodLog_lock; // a lock on allocation of LogExecutedMethods info extern Mutex* RetData_lock; // a lock on installation of RetData inside method data extern Monitor* VMOperation_lock; // a lock on queue of vm_operations waiting to execute +extern Monitor* ThreadsLockThrottle_lock; // used by Thread start/exit to reduce competition for Threads_lock, + // so a VM thread calling a safepoint is prioritized extern Monitor* Threads_lock; // a lock on the Threads table of active Java threads // (also used by Safepoints too to block threads creation/destruction) extern Mutex* NonJavaThreadsList_lock; // a lock on the NonJavaThreads list diff --git a/src/hotspot/share/runtime/threads.cpp b/src/hotspot/share/runtime/threads.cpp index ac435317d05..8e17010e4ab 100644 --- a/src/hotspot/share/runtime/threads.cpp +++ b/src/hotspot/share/runtime/threads.cpp @@ -1017,7 +1017,9 @@ void Threads::add(JavaThread* p, bool force_daemon) { void Threads::remove(JavaThread* p, bool is_daemon) { // Extra scope needed for Thread_lock, so we can check // that we do not remove thread without safepoint code notice - { MonitorLocker ml(Threads_lock); + { + ConditionalMutexLocker throttle_ml(ThreadsLockThrottle_lock, UseThreadsLockThrottleLock); + MonitorLocker ml(Threads_lock); if (ThreadIdTable::is_initialized()) { // This cleanup must be done before the current thread's GC barrier @@ -1065,7 +1067,7 @@ void Threads::remove(JavaThread* p, bool is_daemon) { // Notify threads waiting in EscapeBarriers EscapeBarrier::thread_removed(p); - } // unlock Threads_lock + } // unlock Threads_lock and ThreadsLockThrottle_lock // Reduce the ObjectMonitor ceiling for the exiting thread. ObjectSynchronizer::dec_in_use_list_ceiling(); From 23e3103ba3a4a05be42a10110ecdb17f5affda97 Mon Sep 17 00:00:00 2001 From: Sergey Nazarkin Date: Fri, 24 Jan 2025 13:13:39 +0200 Subject: [PATCH 2/3] Fix absence of ConditionalMutexLocker --- src/hotspot/share/prims/jvm.cpp | 2 +- src/hotspot/share/runtime/threads.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/hotspot/share/prims/jvm.cpp b/src/hotspot/share/prims/jvm.cpp index 958cbe72828..2615c3729be 100644 --- a/src/hotspot/share/prims/jvm.cpp +++ b/src/hotspot/share/prims/jvm.cpp @@ -2955,7 +2955,7 @@ JVM_ENTRY(void, JVM_StartThread(JNIEnv* env, jobject jthread)) // We must release the Threads_lock before we can post a jvmti event // in Thread::start. { - ConditionalMutexLocker throttle_ml(ThreadsLockThrottle_lock, UseThreadsLockThrottleLock); + MutexLocker throttle_ml(UseThreadsLockThrottleLock ? ThreadsLockThrottle_lock : NULL); // Ensure that the C++ Thread and OSThread structures aren't freed before // we operate. MutexLocker ml(Threads_lock); diff --git a/src/hotspot/share/runtime/threads.cpp b/src/hotspot/share/runtime/threads.cpp index 8e17010e4ab..fccea73937d 100644 --- a/src/hotspot/share/runtime/threads.cpp +++ b/src/hotspot/share/runtime/threads.cpp @@ -1018,7 +1018,7 @@ void Threads::remove(JavaThread* p, bool is_daemon) { // Extra scope needed for Thread_lock, so we can check // that we do not remove thread without safepoint code notice { - ConditionalMutexLocker throttle_ml(ThreadsLockThrottle_lock, UseThreadsLockThrottleLock); + MutexLocker throttle_ml(UseThreadsLockThrottleLock ? ThreadsLockThrottle_lock : NULL); MonitorLocker ml(Threads_lock); if (ThreadIdTable::is_initialized()) { From b16fa39b09d5124fc893e281235de87339fdb5cd Mon Sep 17 00:00:00 2001 From: Sergey Nazarkin Date: Tue, 28 Jan 2025 19:14:41 +0200 Subject: [PATCH 3/3] NULL -> nullptr --- src/hotspot/share/prims/jvm.cpp | 2 +- src/hotspot/share/runtime/threads.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/hotspot/share/prims/jvm.cpp b/src/hotspot/share/prims/jvm.cpp index 2615c3729be..fc650e0668c 100644 --- a/src/hotspot/share/prims/jvm.cpp +++ b/src/hotspot/share/prims/jvm.cpp @@ -2955,7 +2955,7 @@ JVM_ENTRY(void, JVM_StartThread(JNIEnv* env, jobject jthread)) // We must release the Threads_lock before we can post a jvmti event // in Thread::start. { - MutexLocker throttle_ml(UseThreadsLockThrottleLock ? ThreadsLockThrottle_lock : NULL); + MutexLocker throttle_ml(UseThreadsLockThrottleLock ? ThreadsLockThrottle_lock : nullptr); // Ensure that the C++ Thread and OSThread structures aren't freed before // we operate. MutexLocker ml(Threads_lock); diff --git a/src/hotspot/share/runtime/threads.cpp b/src/hotspot/share/runtime/threads.cpp index fccea73937d..3d88baae622 100644 --- a/src/hotspot/share/runtime/threads.cpp +++ b/src/hotspot/share/runtime/threads.cpp @@ -1018,7 +1018,7 @@ void Threads::remove(JavaThread* p, bool is_daemon) { // Extra scope needed for Thread_lock, so we can check // that we do not remove thread without safepoint code notice { - MutexLocker throttle_ml(UseThreadsLockThrottleLock ? ThreadsLockThrottle_lock : NULL); + MutexLocker throttle_ml(UseThreadsLockThrottleLock ? ThreadsLockThrottle_lock : nullptr); MonitorLocker ml(Threads_lock); if (ThreadIdTable::is_initialized()) {