RFR: 8305670: Performance regression in LockSupport.unpark with lots of idle threads [v2]

[Daniel D. Daugherty]

On Wed, 19 Apr 2023 16:21:35 GMT, Robbin Ehn <rehn at openjdk.org> wrote:

>> src/hotspot/share/prims/unsafe.cpp line 794:
>> 
>>> 792:         // before and after creating tlh above so quick_mode can be used in
>>> 793:         // this conversion call.
>>> 794:         if (tlh.cv_internal_thread_to_JavaThread(jthread, &thr, nullptr, true /* quick_mode */)) {
>> 
>> Sorry I'm having a mental blank here now. Why do we even need to call this if we can access `java_lang_Thread::thread(thread_oop)` after the TLH was created?
>
> As a caller of cv_internal_thread_to_JavaThread we do not know the implementation of quick_mode.
> 
> As the implementation is now, we still want the asserts.

In particular we want this assert from `cv_internal_thread_to_JavaThread()`:

  assert(includes(java_thread), "must be");

because that verifies our assumptions from this call-site that `quick_mode` is okay.

Also, it is possible that a JavaThread that's racing to exit will make it past `ensure_join()`
after our check on L789 above and run into this block in the conversion function:

  JavaThread *java_thread = java_lang_Thread::thread(thread_oop);
  if (java_thread == nullptr) {
    // The java.lang.Thread does not contain a JavaThread* so it has not
    // run enough to be put on a ThreadsList or it has exited enough to
    // make it past ensure_join() where the JavaThread* is cleared.
    return false;
  }


Because we (the observer thread) captured the target JavaThread in the
ThreadsListHandle, it's safe for us to use the `JavaThread*`, but I've always
been a fan of bailing on operations like these as soon as we detect that the
`JavaThread*` is exiting.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/13519#discussion_r1171642218