RFR: 8223312: Utilize handshakes instead of is_thread_fully_suspended

[David Holmes]

On Mon, 19 Oct 2020 09:59:34 GMT, Robbin Ehn <rehn at openjdk.org> wrote:

> The main point of this change-set is to make it easier to implement S/R on top of handshakes.
> Which is a prerequisite for removing _suspend_flag (which duplicates the handshake functionality).
> But we also remove some complicated S/R methods.
> 
> We basically just put in everything in the handshake closure, so the diff just looks much worse than what it is.
> 
> TraceSuspendDebugBits have an ifdef, but in both cases it now just returns.
> But I was unsure if I should remove now or when is_ext_suspend_completed() is removed.
> 
> Passes multiple t1-5 runs, locally it passes many jck:vm/nsk_jvmti/nsk_jdi/jdk-jdi runs.

Hi Robbin,

IIUC the "waiting" part of `wait_for_ext_suspend_completion` is now implicitly handled in the handshake - correct?

Overall this seems like a great simplification.

A few minor comments below.

Thanks,
David

src/hotspot/share/runtime/thread.cpp line 579:

> 577:     // That trace is very chatty.
> 578:     return;
> 579: #else

Without the !is_wait check none of the code below line 583 is reachable now. I would remove this now.

src/hotspot/share/prims/jvmtiEnv.cpp line 1648:

> 1646:     op.doit(java_thread, true);
> 1647:   } else {
> 1648:     Handshake::execute(&op, java_thread);

This pattern is repeated a lot - we should be able to incorporate it into the op itself by passing in `java_thread`.

src/hotspot/share/prims/jvmtiEnvBase.cpp line 1525:

> 1523:   Thread* current_thread  = Thread::current();
> 1524:   HandleMark hm(current_thread);
> 1525:   JavaThread* java_thread = target->as_Java_thread();

Contrast with the same three lines at L1390 - we should use the same boilerplate in each `doit`. And ideally refactor
into some shared code somewhere (future RFE).

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

Marked as reviewed by dholmes (Reviewer).

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