8249004: Reduce ThreadsListHandle overhead in relation to direct handshakes

Reviewed-by: coleenp, sspitsyn, dholmes, rehn

src/hotspot/share/prims/jvmtiEnv.cpp

@@ -1560,7 +1560,7 @@ JvmtiEnv::GetThreadListStackTraces(jint thread_count, const jthread* thread_list
     }
 
     GetSingleStackTraceClosure op(this, current_thread, *thread_list, max_frame_count);
-    Handshake::execute(&op, java_thread);
+    Handshake::execute(&op, &tlh, java_thread);
     err = op.result();
     if (err == JVMTI_ERROR_NONE) {
       *stack_info_ptr = op.stack_info();

src/hotspot/share/prims/jvmtiEventController.cpp

@@ -616,8 +616,18 @@ JvmtiEventControllerPrivate::recompute_enabled() {
   }
 
   // compute and set thread-filtered events
-  for (JvmtiThreadState *state = JvmtiThreadState::first(); state != NULL; state = state->next()) {
-    any_env_thread_enabled |= recompute_thread_enabled(state);
+  JvmtiThreadState *state = JvmtiThreadState::first();
+  if (state != nullptr) {
+    // If we have a JvmtiThreadState, then we've reached the point where
+    // threads can exist so create a ThreadsListHandle to protect them.
+    // The held JvmtiThreadState_lock prevents exiting JavaThreads from
+    // being removed from the JvmtiThreadState list we're about to walk
+    // so this ThreadsListHandle exists just to satisfy the lower level sanity
+    // checks that the target JavaThreads are protected.
+    ThreadsListHandle tlh;
+    for (; state != nullptr; state = state->next()) {
+      any_env_thread_enabled |= recompute_thread_enabled(state);
+    }
   }
 
   // set universal state (across all envs and threads)

src/hotspot/share/prims/whitebox.cpp

@@ -2070,10 +2070,13 @@ WB_ENTRY(jboolean, WB_HandshakeReadMonitors(JNIEnv* env, jobject wb, jobject thr
   };
 
   ReadMonitorsClosure rmc;
-  oop thread_oop = JNIHandles::resolve(thread_handle);
-  if (thread_oop != NULL) {
-    JavaThread* target = java_lang_Thread::thread(thread_oop);
-    Handshake::execute(&rmc, target);
+  if (thread_handle != NULL) {
+    ThreadsListHandle tlh;
+    JavaThread* target = nullptr;
+    bool is_alive = tlh.cv_internal_thread_to_JavaThread(thread_handle, &target, NULL);
+    if (is_alive) {
+      Handshake::execute(&rmc, &tlh, target);
+    }
   }
   return rmc.executed();
 WB_END
@@ -2101,11 +2104,12 @@ WB_ENTRY(jint, WB_HandshakeWalkStack(JNIEnv* env, jobject wb, jobject thread_han
 
   if (all_threads) {
     Handshake::execute(&tsc);
-  } else {
-    oop thread_oop = JNIHandles::resolve(thread_handle);
-    if (thread_oop != NULL) {
-      JavaThread* target = java_lang_Thread::thread(thread_oop);
-      Handshake::execute(&tsc, target);
+  } else if (thread_handle != NULL) {
+    ThreadsListHandle tlh;
+    JavaThread* target = nullptr;
+    bool is_alive = tlh.cv_internal_thread_to_JavaThread(thread_handle, &target, NULL);
+    if (is_alive) {
+      Handshake::execute(&tsc, &tlh, target);
     }
   }
   return tsc.num_threads_completed();
@@ -2129,11 +2133,14 @@ WB_ENTRY(void, WB_AsyncHandshakeWalkStack(JNIEnv* env, jobject wb, jobject threa
   public:
     TraceSelfClosure(JavaThread* self_target) : AsyncHandshakeClosure("WB_TraceSelf"), _self(self_target) {}
   };
-  oop thread_oop = JNIHandles::resolve(thread_handle);
-  if (thread_oop != NULL) {
-    JavaThread* target = java_lang_Thread::thread(thread_oop);
-    TraceSelfClosure* tsc = new TraceSelfClosure(target);
-    Handshake::execute(tsc, target);
+  if (thread_handle != NULL) {
+    ThreadsListHandle tlh;
+    JavaThread* target = nullptr;
+    bool is_alive = tlh.cv_internal_thread_to_JavaThread(thread_handle, &target, NULL);
+    if (is_alive) {
+      TraceSelfClosure* tsc = new TraceSelfClosure(target);
+      Handshake::execute(tsc, target);
+    }
   }
 WB_END
 

src/hotspot/share/runtime/handshake.cpp

@@ -342,13 +342,23 @@ void Handshake::execute(HandshakeClosure* hs_cl) {
 }
 
 void Handshake::execute(HandshakeClosure* hs_cl, JavaThread* target) {
+  // tlh == nullptr means we rely on a ThreadsListHandle somewhere
+  // in the caller's context (and we sanity check for that).
+  Handshake::execute(hs_cl, nullptr, target);
+}
+
+void Handshake::execute(HandshakeClosure* hs_cl, ThreadsListHandle* tlh, JavaThread* target) {
   JavaThread* self = JavaThread::current();
   HandshakeOperation op(hs_cl, target, Thread::current());
 
   jlong start_time_ns = os::javaTimeNanos();
 
-  ThreadsListHandle tlh;
-  if (tlh.includes(target)) {
+  guarantee(target != nullptr, "must be");
+  if (tlh == nullptr) {
+    guarantee(Thread::is_JavaThread_protected(target, /* checkTLHOnly */ true),
+              "missing ThreadsListHandle in calling context.");
+    target->handshake_state()->add_operation(&op);
+  } else if (tlh->includes(target)) {
     target->handshake_state()->add_operation(&op);
   } else {
     char buf[128];
@@ -396,13 +406,19 @@ void Handshake::execute(AsyncHandshakeClosure* hs_cl, JavaThread* target) {
   jlong start_time_ns = os::javaTimeNanos();
   AsyncHandshakeOperation* op = new AsyncHandshakeOperation(hs_cl, target, start_time_ns);
 
-  ThreadsListHandle tlh;
-  if (tlh.includes(target)) {
-    target->handshake_state()->add_operation(op);
-  } else {
-    log_handshake_info(start_time_ns, op->name(), 0, 0, "(thread dead)");
-    delete op;
+  guarantee(target != nullptr, "must be");
+
+  Thread* current = Thread::current();
+  if (current != target) {
+    // Another thread is handling the request and it must be protecting
+    // the target.
+    guarantee(Thread::is_JavaThread_protected(target, /* checkTLHOnly */ true),
+              "missing ThreadsListHandle in calling context.");
   }
+  // Implied else:
+  // The target is handling the request itself so it can't be dead.
+
+  target->handshake_state()->add_operation(op);
 }
 
 HandshakeState::HandshakeState(JavaThread* target) :

src/hotspot/share/runtime/handshake.hpp

@@ -36,6 +36,7 @@ class HandshakeOperation;
 class JavaThread;
 class SuspendThreadHandshake;
 class ThreadSelfSuspensionHandshake;
+class ThreadsListHandle;
 
 // A handshake closure is a callback that is executed for a JavaThread
 // while it is in a safepoint/handshake-safe state. Depending on the
@@ -64,7 +65,16 @@ class Handshake : public AllStatic {
  public:
   // Execution of handshake operation
   static void execute(HandshakeClosure*       hs_cl);
+  // This version of execute() relies on a ThreadListHandle somewhere in
+  // the caller's context to protect target (and we sanity check for that).
   static void execute(HandshakeClosure*       hs_cl, JavaThread* target);
+  // This version of execute() is used when you have a ThreadListHandle in
+  // hand and are using it to protect target. If tlh == nullptr, then we
+  // sanity check for a ThreadListHandle somewhere in the caller's context
+  // to verify that target is protected.
+  static void execute(HandshakeClosure*       hs_cl, ThreadsListHandle* tlh, JavaThread* target);
+  // This version of execute() relies on a ThreadListHandle somewhere in
+  // the caller's context to protect target (and we sanity check for that).
   static void execute(AsyncHandshakeClosure*  hs_cl, JavaThread* target);
 };
 

src/hotspot/share/runtime/thread.cpp

@@ -435,30 +435,35 @@ void Thread::check_for_dangling_thread_pointer(Thread *thread) {
 }
 #endif
 
-// Is the target JavaThread protected by the calling Thread
-// or by some other mechanism:
-bool Thread::is_JavaThread_protected(const JavaThread* p) {
-  // Do the simplest check first:
-  if (SafepointSynchronize::is_at_safepoint()) {
-    // The target is protected since JavaThreads cannot exit
-    // while we're at a safepoint.
-    return true;
-  }
-
-  // If the target hasn't been started yet then it is trivially
-  // "protected". We assume the caller is the thread that will do
-  // the starting.
-  if (p->osthread() == NULL || p->osthread()->get_state() <= INITIALIZED) {
-    return true;
-  }
-
-  // Now make the simple checks based on who the caller is:
+// Is the target JavaThread protected by the calling Thread or by some other
+// mechanism? If checkTLHOnly is true (default is false), then we only check
+// if the target JavaThread is protected by a ThreadsList (if any) associated
+// with the calling Thread.
+//
+bool Thread::is_JavaThread_protected(const JavaThread* p, bool checkTLHOnly) {
   Thread* current_thread = Thread::current();
-  if (current_thread == p || Threads_lock->owner() == current_thread) {
-    // Target JavaThread is self or calling thread owns the Threads_lock.
-    // Second check is the same as Threads_lock->owner_is_self(),
-    // but we already have the current thread so check directly.
-    return true;
+  if (!checkTLHOnly) {
+    // Do the simplest check first:
+    if (SafepointSynchronize::is_at_safepoint()) {
+      // The target is protected since JavaThreads cannot exit
+      // while we're at a safepoint.
+      return true;
+    }
+
+    // If the target hasn't been started yet then it is trivially
+    // "protected". We assume the caller is the thread that will do
+    // the starting.
+    if (p->osthread() == NULL || p->osthread()->get_state() <= INITIALIZED) {
+      return true;
+    }
+
+    // Now make the simple checks based on who the caller is:
+    if (current_thread == p || Threads_lock->owner() == current_thread) {
+      // Target JavaThread is self or calling thread owns the Threads_lock.
+      // Second check is the same as Threads_lock->owner_is_self(),
+      // but we already have the current thread so check directly.
+      return true;
+    }
   }
 
   // Check the ThreadsLists associated with the calling thread (if any)
@@ -471,16 +476,18 @@ bool Thread::is_JavaThread_protected(const JavaThread* p) {
     }
   }
 
-  // Use this debug code with -XX:+UseNewCode to diagnose locations that
-  // are missing a ThreadsListHandle or other protection mechanism:
-  // guarantee(!UseNewCode, "current_thread=" INTPTR_FORMAT " is not protecting p="
-  //           INTPTR_FORMAT, p2i(current_thread), p2i(p));
+  if (!checkTLHOnly) {
+    // Use this debug code with -XX:+UseNewCode to diagnose locations that
+    // are missing a ThreadsListHandle or other protection mechanism:
+    // guarantee(!UseNewCode, "current_thread=" INTPTR_FORMAT " is not protecting p="
+    //           INTPTR_FORMAT, p2i(current_thread), p2i(p));
 
-  // Note: Since 'p' isn't protected by a TLH, the call to
-  // p->is_handshake_safe_for() may crash, but we have debug bits so
-  // we'll be able to figure out what protection mechanism is missing.
-  assert(p->is_handshake_safe_for(current_thread), "JavaThread=" INTPTR_FORMAT
-         " is not protected and not handshake safe.", p2i(p));
+    // Note: Since 'p' isn't protected by a TLH, the call to
+    // p->is_handshake_safe_for() may crash, but we have debug bits so
+    // we'll be able to figure out what protection mechanism is missing.
+    assert(p->is_handshake_safe_for(current_thread), "JavaThread=" INTPTR_FORMAT
+           " is not protected and not handshake safe.", p2i(p));
+  }
 
   // The target JavaThread is not protected so it is not safe to query:
   return false;
@@ -1743,20 +1750,14 @@ void JavaThread::send_thread_stop(oop java_throwable)  {
 //   - Target thread will not enter any new monitors.
 //
 bool JavaThread::java_suspend() {
-  ThreadsListHandle tlh;
-  if (!tlh.includes(this)) {
-    log_trace(thread, suspend)("JavaThread:" INTPTR_FORMAT " not on ThreadsList, no suspension", p2i(this));
-    return false;
-  }
+  guarantee(Thread::is_JavaThread_protected(this, /* checkTLHOnly */ true),
+            "missing ThreadsListHandle in calling context.");
   return this->handshake_state()->suspend();
 }
 
 bool JavaThread::java_resume() {
-  ThreadsListHandle tlh;
-  if (!tlh.includes(this)) {
-    log_trace(thread, suspend)("JavaThread:" INTPTR_FORMAT " not on ThreadsList, nothing to resume", p2i(this));
-    return false;
-  }
+  guarantee(Thread::is_JavaThread_protected(this, /* checkTLHOnly */ true),
+            "missing ThreadsListHandle in calling context.");
   return this->handshake_state()->resume();
 }
 

src/hotspot/share/runtime/thread.hpp

@@ -199,9 +199,11 @@ class Thread: public ThreadShadow {
   }
 
  public:
-  // Is the target JavaThread protected by the calling Thread
-  // or by some other mechanism:
-  static bool is_JavaThread_protected(const JavaThread* p);
+  // Is the target JavaThread protected by the calling Thread or by some other
+  // mechanism? If checkTLHOnly is true (default is false), then we only check
+  // if the target JavaThread is protected by a ThreadsList (if any) associated
+  // with the calling Thread.
+  static bool is_JavaThread_protected(const JavaThread* p, bool checkTLHOnly = false);
 
   void* operator new(size_t size) throw() { return allocate(size, true); }
   void* operator new(size_t size, const std::nothrow_t& nothrow_constant) throw() {