RFR: 8223312: Utilize handshakes instead of is_thread_fully_suspended [v3]

[Robbin Ehn]

On Wed, 21 Oct 2020 16:47:39 GMT, Daniel D. Daugherty <dcubed at openjdk.org> wrote:

>> src/hotspot/share/prims/jvmtiEnv.cpp line 1808:
>> 
>>> 1806:     }
>>> 1807:     if (java_lang_Class::is_primitive(k_mirror)) {
>>> 1808:       return JVMTI_ERROR_NONE;
>> 
>> The call of JvmtiSuspendControl::print() seems to be eliminated. Ok for me.
>
> It's not clear to me why the `JvmtiSuspendControl::print()` is being
> eliminated. Please explain. The `TraceJVMTICalls` support is so that
> someone can diagnose what JVM/TI calls are being made, including
> context in some cases, so it seems wrong to delete this call.

TraceJVMTICalls is a define local to the file jvmtiEnv.cpp set to false which added some more logging for three of the JVM TI functions.
You must first read the code to see if TraceJVMTICalls affects the functions you have an issue with and then change it to true if your lucky it's on of those three. And then you need to recompile. Before commit you need to set to false again.
Why not just temporary add the JvmtiSuspendControl::print()/relevant logging instead ?
Which you still need to do if it's not one of those three functions.

Since this code is not in jvmtiEnv.cpp, we also would need to move TraceJVMTICalls to global scope in some header.
Turning TraceJVMTICalls into UL is good idea I guess, but not in scope of this :)

>> src/hotspot/share/runtime/thread.cpp line 537:
>> 
>>> 535: // cancelled). Returns true if the thread is externally suspended and
>>> 536: // false otherwise.
>>> 537: bool JavaThread::is_ext_suspend_completed() {
>> 
>> I'd think `JavaThread::is_ext_suspend_completed` can be removed also (as a separate enhancement). It also duplicates code of the handshake mechanism. Just replace VM_ThreadSuspend with a handshake.
>
> `is_ext_suspend_completed()` includes code that detects that a thread
> that is in `_thread_in_native_trans` and does not yet have a walkable
> stack has not completed suspension and we will do some retries in
> this function until the target thread gets stable. We have to make sure
> that the handshake mechanism has a similar stability guarantee or a
> stack walker may fail intermittently.

Handshake can only be executed at safepoint poll site, which means if the stack is walkable in all safepoints it is also true for handshakes. And we would be in so much trouble if it were not walkable in all safepoints :)

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

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