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 serviceability-dev
mailing list