RFR: 8223312: Utilize handshakes instead of is_thread_fully_suspended

[David Holmes]

On 20/10/2020 5:20 pm, Robbin Ehn wrote:
> 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.

Not clear to me this is so generic that it applies to Handshake::execute 
rather than being part of the operation. ??

David
-----

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