RFR: 8319244: implement JVMTI handshakes support for virtual threads [v8]

[Serguei Spitsyn]

On Sat, 18 Nov 2023 14:35:58 GMT, Serguei Spitsyn <sspitsyn at openjdk.org> wrote:

>> src/hotspot/share/prims/jvmtiEnvBase.cpp line 631:
>> 
>>> 629:   return !jdk_internal_vm_Continuation::done(cont) &&
>>> 630:          java_lang_VirtualThread::state(vt) != java_lang_VirtualThread::NEW &&
>>> 631:          java_lang_VirtualThread::state(vt) != java_lang_VirtualThread::TERMINATED;
>> 
>> AFAIU `jdk_internal_vm_Continuation::done(cont)` is correct check that vthread is terminated and works for both mounted and unmounted vthreads.
>> Then `java_lang_VirtualThread::state(vt) != java_lang_VirtualThread::TERMINATED` check is not needed
>
> Good suggestion, thanks!

Added the fix suggested by Alex.

>> src/hotspot/share/prims/jvmtiEnvBase.cpp line 1989:
>> 
>>> 1987:     } else {
>>> 1988:       Handshake::execute(hs_cl, tlh, target_jt); // delegate to Handshake implementation
>>> 1989:     }
>> 
>> Every implementation of JvmtiUnitedHandshakeClosure has to check if the target thread is virtual and call do_vthread manually.
>> I'd suggest to handle this by proxy class, something like
>> Suggestion:
>> 
>>     class Adapter : public HandshakeClosure {
>>       JvmtiUnitedHandshakeClosure* _hs_cl;
>>       Handle _target_h;
>>     public:
>>       Adapter(JvmtiUnitedHandshakeClosure* hs_cl, Handle target_h)
>>           : HandshakeClosure(hs_cl->name()), _hs_cl(hs_cl), _target_h(target_h) {}
>>       virtual void do_thread(Thread* thread) {
>>         if (java_lang_VirtualThread::is_instance(_target_h())) { // virtual thread
>>           _hs_cl->do_vthread(_target_h);
>>         } else {
>>           _hs_cl->do_thread(target);
>>         }
>>       }
>>     } adapter(hs_cl, target_h);
>> 
>>     if (self) {                    // target thread is current
>>       adapter.do_thread(target_jt); // execute handshake closure callback on current thread directly
>>     } else {
>>       Handshake::execute(&adapter, tlh, target_jt); // delegate to Handshake implementation
>>     }
>
> Thank you for the suggestion! Agreed, this should help to get rid of this duplication/ugliness.

Added the fix suggested by Alex.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16460#discussion_r1398324679
PR Review Comment: https://git.openjdk.org/jdk/pull/16460#discussion_r1398324619