On Fri, 17 Nov 2023 05:37:46 GMT, Serguei Spitsyn <sspit...@openjdk.org> wrote:

>> The handshakes support for virtual threads is needed to simplify the JVMTI 
>> implementation for virtual threads. There is a significant duplication in 
>> the JVMTI code to differentiate code intended to support platform, virtual 
>> threads or both. The handshakes are unified, so it is enough to define just 
>> one handshake for both platform and virtual thread.
>> At the low level, the JVMTI code supporting platform and virtual threads 
>> still can be different.
>> This implementation is based on the `JvmtiVTMSTransitionDisabler` class.
>> 
>> The internal API includes two new classes:
>>   - `JvmtiHandshake` and `JvmtiUnifiedHandshakeClosure`
>>   
>>   The `JvmtiUnifiedHandshakeClosure` defines two different callback 
>> functions: `do_thread()` and `do_vthread()`.
>> 
>> The first JVMTI functions are picked first to be converted to use the 
>> `JvmtiHandshake`:
>>  - `GetStackTrace`, `GetFrameCount`, `GetFrameLocation`, `NotifyFramePop`
>> 
>> To get the test results clean, the update also fixes the test issue:
>> [8318631](https://bugs.openjdk.org/browse/JDK-8318631): 
>> GetStackTraceSuspendedStressTest.java failed with "check_jvmti_status: JVMTI 
>> function returned error: JVMTI_ERROR_THREAD_NOT_ALIVE (15)" 
>> 
>> Testing:
>>  - the mach5 tiers 1-6 are all passed
>
> Serguei Spitsyn has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   review: add jdk_internal_vm_Continuation::done(cont) check to 
> JvmtiEnvBase::is_vthread_alive

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
    }

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

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

Reply via email to