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

Daniel D. Daugherty dcubed at openjdk.org
Thu Apr 20 17:40:45 UTC 2023


On Thu, 20 Apr 2023 01:19:15 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> Perhaps the comment should change to:
>> 
>>       ThreadsListHandle tlh; // Also is a barrier between the two JavaThread* queries.
>
> I agree we need these ordering constraints, but I'm not clear what is actually guaranteeing them on either the reader or writer side. I imagine there are a number of "barriers" in relation to TLH construction, but as adding to the threadsList is done under a lock (as is setting eetop) then I'm not sure what explicit barriers exist there. Maybe `java_lang_Thread::set_thread` needs to use `release_address_field_put`?

There are ~76 ThreadsListHandle variables in the code base. These are the only ones with comments:

src/hotspot/share/runtime/threadSMR.cpp:    ThreadsListHandle tlh;  // make the current ThreadsList safe for reporting
src/hotspot/share/prims/unsafe.cpp:      ThreadsListHandle tlh; // Provides memory barrier
src/hotspot/share/compiler/compileBroker.cpp:        ThreadsListHandle tlh;  // name() depends on the TLH.
src/hotspot/share/compiler/compileBroker.cpp:        ThreadsListHandle tlh;  // name() depends on the TLH.
src/hotspot/share/compiler/compileBroker.cpp:        ThreadsListHandle tlh;  // name() depends on the TLH.
src/hotspot/share/compiler/compileBroker.cpp:        ThreadsListHandle tlh;  // name() depends on the TLH.


The new "// Provides memory barrier" is the only one that even talks about barriers.
I'm inclined to delete the comment given that these current lines of code:

    if (java_lang_Thread::thread(thread_oop) != nullptr) {
      // Try to capture the live JavaThread in a ThreadsListHandle:
      ThreadsListHandle tlh;
      if (java_lang_Thread::thread(thread_oop) != nullptr) {
        // The still live JavaThread is protected by the ThreadsListHandle.
        JavaThread* thr = nullptr;

I think the comments added in the previous comment are enough to make this code clear.
@robehn and @dholmes-ora - are you both good with this?

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

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


More information about the hotspot-runtime-dev mailing list