RFR: 8308400: add ForceEarlyReturn support for virtual threads [v2]

Leonid Mesnik lmesnik at openjdk.org
Sat May 20 16:08:50 UTC 2023


On Sat, 20 May 2023 00:21:04 GMT, Serguei Spitsyn <sspitsyn at openjdk.org> wrote:

>> This enhancement adds ForceEarlyReturnXXX support for virtual threads. The spec defines minimal support that the JVMTI ForceEarlyReturnXXX can be used for a virtual thread suspended an an event.
>> Actually, the ForceEarlyReturnXXX can supports suspended and mounted virtual threads.
>> 
>> CSR (approved): https://bugs.openjdk.org/browse/JDK-8308401 add ForceEarlyReturn support for virtual threads
>> 
>> Testing:
>> New test was developed: serviceability/vthread/ForceEarlyReturnTest.
>> Submitted mach5 tiers 1-6 are good.
>> TBD: rerun mach5 tiers 1-6 at the end of review again if necessary.
>
> Serguei Spitsyn has updated the pull request incrementally with one additional commit since the last revision:
> 
>   minor tweak in libForceEarlyReturnTest.cpp

The overall looks good. A couple of comments inline.
Although, test is very similar to popframe tests, seems merging code doesn't give a lot of benefits,

The overall looks good. A couple of comments inline.
Although, test is very similar to popframe tests, seems merging code doesn't give a lot of benefits,

src/hotspot/share/prims/jvmtiEnvBase.cpp line 2042:

> 2040:     return err;
> 2041:   }
> 2042:   bool is_virtual = thread_obj != nullptr && thread_obj->is_a(vmClasses::BaseVirtualThread_klass());

Does it make sense to reduce code duplication by moving these checks from forceearlyreturn and popframe code into a separate method?

src/hotspot/share/prims/jvmtiEnvBase.cpp line 2078:

> 2076:     return; /* JVMTI_ERROR_THREAD_NOT_ALIVE (default) */
> 2077:   }
> 2078:   if (!self) {

Can't we have any racing by removing this check?
We are checking thread state before handshake operation, but it is changed before thread start execution of this handshake?

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

Changes requested by lmesnik (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/14067#pullrequestreview-1435420629
PR Review: https://git.openjdk.org/jdk/pull/14067#pullrequestreview-1435421068
PR Review Comment: https://git.openjdk.org/jdk/pull/14067#discussion_r1199629139
PR Review Comment: https://git.openjdk.org/jdk/pull/14067#discussion_r1199629397


More information about the hotspot-dev mailing list