RFR: 8249004: Reduce ThreadsListHandle overhead in relation to direct handshakes [v6]

[Daniel D.Daugherty]

On Fri, 15 Oct 2021 06:34:42 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> Daniel D. Daugherty has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   8249004.cr1.patch
>
> src/hotspot/share/prims/jvmtiEventController.cpp line 623:
> 
>> 621:     // If we have a JvmtiThreadState, then we've reached the point where
>> 622:     // threads can exist so create a ThreadsListHandle to protect them.
>> 623:     ThreadsListHandle tlh;
> 
> Good catch on the missing TLH for this code.

It wasn't quite missing from the baseline code. This version of execute():

`Handshake::execute(HandshakeClosure* hs_cl, JavaThread* target)`

used to always create a ThreadsListHandle. I added a `ThreadsListHandle*`
parameter to that version and created a wrapper with the existing signature
to pass `nullptr` to the execute() version with the `ThreadsListHandle*`
parameter. What that means is that all existing callers of:

`Handshake::execute(HandshakeClosure* hs_cl, JavaThread* target)`

no longer had a ThreadsListHandle created for them. With the new sanity
check in place, I shook the trees to make sure that we had explicit
ThreadsListHandles in place for the locations that needed them.

`JvmtiEventControllerPrivate::recompute_enabled()` happened to be
one of the places where the ThreadsListHandle created by execute()
was hiding the fact that `recompute_enabled()` needed one.

> src/hotspot/share/prims/jvmtiEventController.cpp line 624:
> 
>> 622:     // threads can exist so create a ThreadsListHandle to protect them.
>> 623:     ThreadsListHandle tlh;
>> 624:     for (; state != NULL; state = state->next()) {
> 
> s/NULL/nullptr/

Missed that one. Fixed.

> src/hotspot/share/runtime/handshake.cpp line 361:
> 
>> 359:   } else {
>> 360:     if (tlh_p == nullptr) {
>> 361:       guarantee(Thread::is_JavaThread_protected_by_my_ThreadsList(target),
> 
> This should be an assert once this has had some bake time.

Agreed. All of the `guarantee(Thread::is_JavaThread_protected_by_my_ThreadsList(),...`
calls should be changed to asserts down the road.

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

PR: https://git.openjdk.java.net/jdk/pull/4677