RFR: 8223312: Utilize handshakes instead of is_thread_fully_suspended

Robbin Ehn rehn at openjdk.java.net
Tue Oct 20 07:20:29 UTC 2020


On Tue, 20 Oct 2020 02:22:15 GMT, David Holmes <dholmes 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.
>
> 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`.

My suggestion here is that we fix this in Handshake::execute() in separate RFE.

> 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).

Yes, that would be good.

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

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


More information about the hotspot-dev mailing list