On Tue, 2 Nov 2021 17:26:41 GMT, Daniel D. Daugherty <dcu...@openjdk.org> wrote:
>> A fix to reduce ThreadsListHandle overhead in relation to handshakes and
>> we add sanity checks for ThreadsListHandles higher in the call stack.
>>
>> This fix was tested with Mach5 Tier[1-8]; Tier8 is still running.
>
> Daniel D. Daugherty has updated the pull request incrementally with one
> additional commit since the last revision:
>
> 8249004.cr2.patch
Hi Dan,
Generally seems okay but a couple of minor issues below.
Thanks,
David
src/hotspot/share/runtime/handshake.cpp line 350:
> 348: }
> 349:
> 350: void Handshake::execute(HandshakeClosure* hs_cl, ThreadsListHandle*
> tlh_p, JavaThread* target) {
Nit: can we drop the `_p` part of `tlh_p` please.
src/hotspot/share/runtime/thread.cpp line 446:
> 444: Thread* current_thread = nullptr;
> 445: if (checkTLHOnly) {
> 446: current_thread = Thread::current();
This seems redundant due to line 463. You can just have a `if (!checkTLHOnly)`
block here.
src/hotspot/share/runtime/thread.cpp line 1764:
> 1762: guarantee(Thread::is_JavaThread_protected(this, /* checkTLHOnly */
> true),
> 1763: "missing ThreadsListHandle in calling context.");
> 1764: if (is_exiting()) {
Can't we remove this the same as we did for `java_suspend()`?
-------------
Changes requested by dholmes (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/4677
