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

[Daniel D.Daugherty]

On Fri, 15 Oct 2021 22:19:15 GMT, Daniel D. Daugherty <dcubed at openjdk.org> wrote:

>> So suspend_thread and resume thread's caller already takes a ThreadsListHandle so this is unnecessary and never happens.
>
>> This seems an unrelated change in behaviour ??
> 
> Actually this is equivalent code. The baseline code does this:
> 
>   ThreadsListHandle tlh;
>   if (!tlh.includes(this)) {
>     log_trace(thread, suspend)("JavaThread:" INTPTR_FORMAT " not on ThreadsList, no suspension", p2i(this));
>     return false;
>   }
> 
> 
> What that code means is: if `this` thread does not appear in the newly created
> ThreadsListHandle's list, then `this` thread has called `java_suspend()` after
> `this` thread has exited. That's the only way that `this` thread could be missing
> from a newly created ThreadsListHandle's list.
> 
> All I've done here is get rid of the ThreadsListHandle, verify that the calling
> context is protecting `this` and switch to an `is_exiting()` call.
> 
>> So suspend_thread and resume thread's caller already takes a ThreadsListHandle
>> so this is unnecessary and never happens.
> 
> I presume @coleenp is saying that the `is_exiting()` check is not necessary. 
> 
> That might be true, it might not be true. I was trying to create equivalent
> code without creating a nested ThreadsListHandle here and I think I've
> done that. I actually think it might be possible for a JVM/TI event handler
> to fire after the thread has set is_exiting() and for that event handler to
> call SuspendThread() which could get us to this point.

On rereading all of these comments and the current baseline code, I have
to clarify one thing:

There is a minor change in behavior caused by switching from a
`ThreadsListHandle::includes()` check to a `JavaThread::is_exiting()`
check. The `JavaThread::is_exiting()` will return true before the target
JavaThread is removed from the system's current ThreadsList. So the
`JavaThread::is_exiting()` check will return true faster and the
`ThreadsListHandle::includes()` check.

However, that change in behavior does not result in a change in
behavior for a suspend thread request. Here's the relevant code:


src/hotspot/share/runtime/thread.cpp:
void JavaThread::exit(bool destroy_vm, ExitType exit_type) {
<snip>

    // The careful dance between thread suspension and exit is handled here.
    // Since we are in thread_in_vm state and suspension is done with handshakes,
    // we can just put in the exiting state and it will be correctly handled.
    set_terminated(_thread_exiting);

<snip>

  // Remove from list of active threads list, and notify VM thread if we are the last non-daemon thread
  Threads::remove(this, daemon);

<snip>
}


When we changed the suspend thread mechanism to use handshakes, we
made it so that once the JavaThread called `set_terminated(_thread_exiting)`
it no longer had to honor a suspend thread request.

Summary: Yes, the `is_exiting()` call will result in detecting the exiting
JavaThread sooner than the `ThreadsListHandle::includes()` check.
However, it will not result in a change in suspend thread behavior.

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

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