On Thu, 8 Apr 2021 16:09:52 GMT, Richard Reingruber <rr...@openjdk.org> wrote:
>> Robbin Ehn has updated the pull request with a new target base due to a
>> merge or a rebase. The pull request now contains six commits:
>>
>> - White space fixes
>> - Merge branch 'master' into SuspendInHandshake
>> - Review fixes
>> - Merge branch 'master' into SuspendInHandshake
>> - Merge branch 'master' into SuspendInHandshake
>> - 8257831: Suspend with handshake (review baseline)
>
> Hi Robbin,
>
> thanks for the update. This is almost ready to be pushed from my pov.
>
> Richard.
Hi Robbin,
Just a couple of comments below.
src/hotspot/share/runtime/handshake.hpp
154 // This flag is true while there is async handshake (trap)
155 // on queue. Since we do only need one, we can reuse it if
156 // thread thread gets suspended again (after a resume)
157 // and have not yet processed it.
158 bool _async_suspend_handshake;
A couple of nits above:
L156: "thread thread" => "thread"
L157: "and have not yet processed it." => "and we have not yet processed it."
src/hotspot/share/runtime/handshake.cpp
658 bool HandshakeState::suspend_with_handshake() {
. . .
684 ThreadSuspensionHandshake* ts = new ThreadSuspensionHandshake();
685 Handshake::execute(ts, _handshakee);
I wonder why ThreadSuspensionHandshake is not stack allocated as
SuspendThreadHandshake below:
701 bool HandshakeState::suspend() {
702 SuspendThreadHandshake st;
703 Handshake::execute(&st, _handshakee);
Thanks,
Serguei
-------------
PR: https://git.openjdk.java.net/jdk/pull/3191
