8330969: scalability issue with loaded JVMTI agent

Reviewed-by: amenkov, cjplummer

src/hotspot/share/prims/jvmtiEnvBase.cpp

@@ -1633,19 +1633,16 @@ private:
   }
 
   // This function is called only if _enable == true.
-  // Iterates over all JavaThread's, counts VTMS transitions and restores
+  // Iterates over all JavaThread's, restores jt->jvmti_thread_state() and
-  // jt->jvmti_thread_state() and jt->jvmti_vthread() for VTMS transition protocol.
+  // jt->jvmti_vthread() for VTMS transition protocol.
-  int count_transitions_and_correct_jvmti_thread_states() {
+  void correct_jvmti_thread_states() {
-    int count = 0;
-
     for (JavaThread* jt : ThreadsListHandle()) {
       if (jt->is_in_VTMS_transition()) {
-        count++;
+        jt->set_VTMS_transition_mark(true);
         continue; // no need in JvmtiThreadState correction below if in transition
       }
       correct_jvmti_thread_state(jt);
     }
-    return count;
   }
 
 public:
@@ -1655,9 +1652,9 @@ public:
   }
 
   void doit() {
-    int count = _enable ? count_transitions_and_correct_jvmti_thread_states() : 0;
+    if (_enable) {
-
+      correct_jvmti_thread_states();
-    JvmtiVTMSTransitionDisabler::set_VTMS_transition_count(count);
+    }
     JvmtiVTMSTransitionDisabler::set_VTMS_notify_jvmti_events(_enable);
   }
 };

src/hotspot/share/prims/jvmtiThreadState.cpp

@@ -214,9 +214,6 @@ JvmtiThreadState::periodic_clean_up() {
 // Virtual Threads Mount State transition (VTMS transition) mechanism
 //
 
-// VTMS transitions cannot be disabled while this counter is positive.
-volatile int JvmtiVTMSTransitionDisabler::_VTMS_transition_count = 0;
-
 // VTMS transitions for one virtual thread are disabled while it is positive
 volatile int JvmtiVTMSTransitionDisabler::_VTMS_transition_disable_for_one_count = 0;
 
@@ -238,11 +235,14 @@ volatile bool JvmtiVTMSTransitionDisabler::_sync_protocol_enabled_permanently =
 #ifdef ASSERT
 void
 JvmtiVTMSTransitionDisabler::print_info() {
-  log_error(jvmti)("_VTMS_transition_count: %d\n", _VTMS_transition_count);
   log_error(jvmti)("_VTMS_transition_disable_for_one_count: %d\n", _VTMS_transition_disable_for_one_count);
   log_error(jvmti)("_VTMS_transition_disable_for_all_count: %d\n\n", _VTMS_transition_disable_for_all_count);
   int attempts = 10000;
   for (JavaThreadIteratorWithHandle jtiwh; JavaThread *java_thread = jtiwh.next(); ) {
+    if (java_thread->VTMS_transition_mark()) {
+      log_error(jvmti)("jt: %p VTMS_transition_mark: %d\n",
+                       (void*)java_thread, java_thread->VTMS_transition_mark());
+    }
     ResourceMark rm;
     // Handshake with target.
     PrintStackTraceClosure pstc;
@@ -358,11 +358,13 @@ JvmtiVTMSTransitionDisabler::VTMS_transition_disable_for_all() {
 
     // Block while some mount/unmount transitions are in progress.
     // Debug version fails and prints diagnostic information.
-    while (_VTMS_transition_count > 0) {
+    for (JavaThreadIteratorWithHandle jtiwh; JavaThread *jt = jtiwh.next(); ) {
-      if (ml.wait(10)) {
+      while (jt->VTMS_transition_mark()) {
-        attempts--;
+        if (ml.wait(10)) {
+          attempts--;
+        }
+        DEBUG_ONLY(if (attempts == 0) break;)
       }
-      DEBUG_ONLY(if (attempts == 0) break;)
     }
     assert(!thread->is_VTMS_transition_disabler(), "VTMS_transition sanity check");
 #ifdef ASSERT
@@ -427,8 +429,9 @@ JvmtiVTMSTransitionDisabler::start_VTMS_transition(jthread vthread, bool is_moun
   assert(!thread->is_in_VTMS_transition(), "VTMS_transition sanity check");
 
   // Avoid using MonitorLocker on performance critical path, use
-  // two-level synchronization with lock-free operations on counters.
+  // two-level synchronization with lock-free operations on state bits.
-  Atomic::inc(&_VTMS_transition_count); // Try to enter VTMS transition section optmistically.
+  assert(!thread->VTMS_transition_mark(), "sanity check");
+  thread->set_VTMS_transition_mark(true); // Try to enter VTMS transition section optmistically.
   java_lang_Thread::set_is_in_VTMS_transition(vt, true);
 
   if (!sync_protocol_enabled()) {
@@ -448,9 +451,9 @@ JvmtiVTMSTransitionDisabler::start_VTMS_transition(jthread vthread, bool is_moun
       thread->is_suspended() ||
       JvmtiVTSuspender::is_vthread_suspended(thread_id)
   ) {
-    // Slow path: undo unsuccessful optimistic counter incrementation.
+    // Slow path: undo unsuccessful optimistic set of the VTMS_transition_mark.
     // It can cause an extra waiting cycle for VTMS transition disablers.
-    Atomic::dec(&_VTMS_transition_count);
+    thread->set_VTMS_transition_mark(false);
     java_lang_Thread::set_is_in_VTMS_transition(vth(), false);
 
     while (true) {
@@ -470,7 +473,7 @@ JvmtiVTMSTransitionDisabler::start_VTMS_transition(jthread vthread, bool is_moun
         DEBUG_ONLY(if (attempts == 0) break;)
         continue;  // ~ThreadBlockInVM has handshake-based suspend point.
       }
-      Atomic::inc(&_VTMS_transition_count);
+      thread->set_VTMS_transition_mark(true);
       java_lang_Thread::set_is_in_VTMS_transition(vth(), true);
       break;
     }
@@ -495,7 +498,8 @@ JvmtiVTMSTransitionDisabler::finish_VTMS_transition(jthread vthread, bool is_mou
   thread->set_is_in_VTMS_transition(false);
   oop vt = JNIHandles::resolve_external_guard(vthread);
   java_lang_Thread::set_is_in_VTMS_transition(vt, false);
-  Atomic::dec(&_VTMS_transition_count);
+  assert(thread->VTMS_transition_mark(), "sanity check");
+  thread->set_VTMS_transition_mark(false);
 
   if (!sync_protocol_enabled()) {
     return;

src/hotspot/share/prims/jvmtiThreadState.hpp

@@ -82,9 +82,8 @@ class JvmtiVTMSTransitionDisabler {
   static volatile int _VTMS_transition_disable_for_one_count; // transitions for one virtual thread are disabled while it is positive
   static volatile int _VTMS_transition_disable_for_all_count; // transitions for all virtual threads are disabled while it is positive
   static volatile bool _SR_mode;                         // there is an active suspender or resumer
-  static volatile int _VTMS_transition_count;            // current number of VTMS transitions
   static volatile int _sync_protocol_enabled_count;      // current number of JvmtiVTMSTransitionDisablers enabled sync protocol
-  static volatile bool _sync_protocol_enabled_permanently; // seen a suspender: JvmtiVTMSTraansitionDisabler protocol is enabled permanently
+  static volatile bool _sync_protocol_enabled_permanently; // seen a suspender: JvmtiVTMSTransitionDisabler protocol is enabled permanently
 
   bool _is_SR;                                           // is suspender or resumer
   jthread _thread;                                       // virtual thread to disable transitions for, no-op if it is a platform thread
@@ -100,8 +99,6 @@ class JvmtiVTMSTransitionDisabler {
   static bool VTMS_notify_jvmti_events()             { return _VTMS_notify_jvmti_events; }
   static void set_VTMS_notify_jvmti_events(bool val) { _VTMS_notify_jvmti_events = val; }
 
-  static void set_VTMS_transition_count(bool val)    { _VTMS_transition_count = val; }
-
   static void inc_sync_protocol_enabled_count()      { Atomic::inc(&_sync_protocol_enabled_count); }
   static void dec_sync_protocol_enabled_count()      { Atomic::dec(&_sync_protocol_enabled_count); }
   static int  sync_protocol_enabled_count()          { return Atomic::load(&_sync_protocol_enabled_count); }

src/hotspot/share/runtime/javaThread.cpp

@@ -450,6 +450,7 @@ JavaThread::JavaThread() :
   _is_in_VTMS_transition(false),
   _is_in_tmp_VTMS_transition(false),
   _is_disable_suspend(false),
+  _VTMS_transition_mark(false),
 #ifdef ASSERT
   _is_VTMS_transition_disabler(false),
 #endif

src/hotspot/share/runtime/javaThread.hpp

@@ -317,6 +317,7 @@ class JavaThread: public Thread {
   bool                  _is_in_VTMS_transition;          // thread is in virtual thread mount state transition
   bool                  _is_in_tmp_VTMS_transition;      // thread is in temporary virtual thread mount state transition
   bool                  _is_disable_suspend;             // JVMTI suspend is temporarily disabled; used on current thread only
+  bool                  _VTMS_transition_mark;           // used for sync between VTMS transitions and disablers
 #ifdef ASSERT
   bool                  _is_VTMS_transition_disabler;    // thread currently disabled VTMS transitions
 #endif
@@ -664,6 +665,9 @@ private:
   bool is_disable_suspend() const                { return _is_disable_suspend; }
   void toggle_is_disable_suspend()               { _is_disable_suspend = !_is_disable_suspend; };
 
+  bool VTMS_transition_mark() const              { return Atomic::load(&_VTMS_transition_mark); }
+  void set_VTMS_transition_mark(bool val)        { Atomic::store(&_VTMS_transition_mark, val); }
+
 #ifdef ASSERT
   bool is_VTMS_transition_disabler() const       { return _is_VTMS_transition_disabler; }
   void set_is_VTMS_transition_disabler(bool val);