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

Daniel D.Daugherty dcubed at openjdk.java.net
Fri Oct 15 22:26:55 UTC 2021

On Fri, 15 Oct 2021 18:20:12 GMT, Coleen Phillimore <coleenp 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/runtime/handshake.cpp line 358:
>> 356:   bool target_is_dead = false;
>> 357:   if (target == nullptr) {
>> 358:     target_is_dead = true;
> Why would you pass a NULL target thread to Handshake::execute? Why would the caller not check if the target is dead?

The `NULL` target thread being passed in is actually handled by the baseline code:

  ThreadsListHandle tlh;
  if (tlh.includes(target)) {

`tlh.includes(target)` returns `false` when `target` is `NULL/nullptr`.
I just made the already handled situation more explicit.

> Why would the caller not check if the target is dead?

Hmmm...  It's hard for me to answer that question since I didn't write
the original code. The test code that calls `WB_HandshakeWalkStack()`
or `WB_AsyncHandshakeWalkStack()` can call those functions with
a `thread_handle` that translates into a `thread_oop` that returns a
`NULL` `JavaThread*`.

See the comment that I added to `WB_AsyncHandshakeWalkStack()` above.

> src/hotspot/share/runtime/thread.cpp line 497:
>> 495: // placement somewhere in the calling context.
>> 496: bool Thread::is_JavaThread_protected_by_my_ThreadsList(const JavaThread* p) {
>> 497:   Thread* current_thread = Thread::current();
> Shouldn't you call this on the current thread as "this" argument?

I modeled the new check after the existing:

bool Thread::is_JavaThread_protected(const JavaThread* p) {

which is also a static function.


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

More information about the hotspot-runtime-dev mailing list