RFR: 8298853: JvmtiVTMSTransitionDisabler should support disabling one virtual thread transitions [v5]

[David Holmes]

On Sat, 24 Dec 2022 04:14:07 GMT, Serguei Spitsyn <sspitsyn at openjdk.org> wrote:

>> Now the `JvmtiVTMSTransitionDisabler` mechanism supports disabling VTMS transitions for all virtual threads only. It should also support disabling transitions for any specific virtual thread as well. This will improve scalability of the JVMTI functions operating on target virtual threads as the functions can be executed concurrently without blocking each other execution when target virtual threads are different.
>> New constructor `JvmtiVTMSTransitionDisabler(jthread vthread)` is added which has jthread parameter of the target virtual thread.
>> 
>> Testing:
>>   mach5 jobs are TBD (preliminary testing was completed):
>>    - all JVMTI, JDWP, JDI and JDB tests have to be run
>>    - Kitchensink
>>    - tier5
>
> Serguei Spitsyn has updated the pull request incrementally with one additional commit since the last revision:
> 
>   fix race between VTMS_transition_disable_for_one and start_VTMS_transition

Still muddling through this ...

src/hotspot/share/classfile/javaClasses.cpp line 1740:

> 1738: void java_lang_Thread::inc_VTMS_transition_disable_count(oop java_thread) {
> 1739:   int val = VTMS_transition_disable_count(java_thread);
> 1740:   assert(JvmtiVTMSTransition_lock->owned_by_self(), "Must be locked");

Nit: normally a lock-checking assertion would come first in the function, so that it stands out more.

src/hotspot/share/classfile/javaClasses.cpp line 1746:

> 1744: void java_lang_Thread::dec_VTMS_transition_disable_count(oop java_thread) {
> 1745:   int val = VTMS_transition_disable_count(java_thread);
> 1746:   assert(JvmtiVTMSTransition_lock->owned_by_self(), "Must be locked");

Nit: normally a lock-checking assertion would come first in the function, so that it stands out more.

src/hotspot/share/prims/jvmtiThreadState.cpp line 384:

> 382:   JvmtiThreadState* vstate = java_lang_Thread::jvmti_thread_state(vth());
> 383:   if (vstate != NULL) {
> 384:     vstate->set_is_in_VTMS_transition(true);

Is the VTMS transition flag in the `JvmtiThreadState` dead code now?

src/hotspot/share/prims/jvmtiThreadState.cpp line 482:

> 480:       _VTMS_transition_disable_for_all_count > 0) {
> 481:     MonitorLocker ml(JvmtiVTMSTransition_lock, Mutex::_no_safepoint_check_flag);
> 482:     ml.notify_all();

Checking the counts outside the lock seems really racy. Maybe it is safe in the original code but now we have two counters it is very hard to ascertain this is correct. Overall I find it very hard to see exactly how these counters get used.

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

PR: https://git.openjdk.org/jdk/pull/11690