[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: RFR: 8249004: Reduce ThreadsListHandle overhead in relation to direct handshakes [v6]
From:       Daniel D.Daugherty <dcubed () openjdk ! java ! net>
Date:       2021-10-29 22:43:14
Message-ID: WWkHiys7JmRTu_cgB4cG7cWgaTu9XzFEyzWhNWz3i0U=.88725a32-b783-4d74-a361-a606d1d2cb54 () github ! com
[Download RAW message or body]

On Sun, 17 Oct 2021 12:52:15 GMT, David Holmes <dholmes@openjdk.org> wrote:

> > 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.
> 
> The `is_exiting` check seems unnecessary as the handshake code will not handshake \
> with an exiting thread. The nested TLH was unnecessary too AFAICS.

Ummmm... The purpose of the new `is_exiting()` check and the baseline's
`ThreadsListHandle::includes()` check is to avoid making this call:

  return this->handshake_state()->suspend();

The call we are avoiding is the one that makes the synchronous
SuspendThreadHandshake request (for threads other than self)
so by detecting the exiting thread early, we are avoiding entering
the handshake machinery.

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

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


[prev in list] [next in list] [prev in thread] [next in thread]