On Thu, 22 Sep 2022 09:16:28 GMT, Serguei Spitsyn <[email protected]> wrote:
>> There are several places in VirtualThread class implementation where virtual
>> threads are being mounted or unmounted, so there is a transition of the
>> JavaThread identity from carrier thread to virtual thread and back. The
>> execution state in such transitions is inconsistent, and so, has to be
>> invisible to JVM TI agents. Such transitions are named as VTMS (virtual
>> thread mount state) transitions, and there is a JVM TI mechanism which
>> supports them. Besides normal VTMS transitions there are also a couple of
>> performance-critical contexts (in `VirtualThread` methods:
>> `scheduleUnpark()`, `cancel()` and `unpark()`) which can be named as
>> temporary VTMS transitions. Execution state of such temporary VTMS
>> transitions has to be also invisible to the JVM TI agent which is
>> implemented in the current update.
>>
>> There are a couple of details of this fix to highlight:
>> - A JavaThread which is in temporary VTMS transition is marked with a
>> special bit `_is_in_tmp_VTMS_transition`. It is done with the native
>> notification method `toggleJvmtiTmpVTMSTrans()`.
>> - A couple of lambda form classes can be created in context of temporary
>> VTMS transitions, so their `ClassLoad`, `ClassPrepare` and `CFLH` events are
>> ignored.
>> - Suspending threads in temporary transition is allowed.
>>
>> The fix was tested in Loom repository by using `JTREG_MAIN_WRAPPER=Virtual`
>> mode of execution which forces all main threads to be run as virtual. All
>> `JVM TI` and `JDI` tests were run on all platforms. It includes
>> `Kitchensink` and `JCK` tests. Additionally, the fix was tested in the jdk
>> 20 repository. It includes `JVM TI` and `JDI` tests and tiers 1-6.
>
> Serguei Spitsyn has updated the pull request incrementally with one
> additional commit since the last revision:
>
> 1. addressed review comments from Chris; added VirtualThread.java update
> from Alan
src/hotspot/share/runtime/javaThread.hpp line 652:
> 650: void set_is_in_VTMS_transition(bool val);
> 651: void toggle_is_in_tmp_VTMS_transition() {
> _is_in_tmp_VTMS_transition = !_is_in_tmp_VTMS_transition; };
> 652:
My suggestion was to have the term "in VTMS transition" be inclusive of temp
transitions. So based on your current names I would suggest:
- is_in_VTMS_transition -> is_in_non_tmp_VTMS_transition
- is_in_any_VTMS_transition -> is_in_VTMS_transition
But that becomes a problem for `set_is_in_VTMS_transition`, which would need to
be renamed `set_is_in_non_tmp_VTMS_transition`, which I'm guessing you don't
want to do. So let's instead just hope this all goes away before thinking about
it any more.
-------------
PR: https://git.openjdk.org/jdk/pull/10321
