RFR: 8298853: JvmtiVTMSTransitionDisabler should support disabling one virtual thread transitions

[David Holmes]

On Thu, 15 Dec 2022 11:51:10 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

Seems logical though I'm not familiar with the existing mechanisms. A few minor comments.

Thanks.

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

> 1743:   int val = VTMS_transition_disable_count(java_thread);
> 1744:   java_thread->int_field_put(_jvmti_VTMS_transition_disable_count_offset, val - 1);
> 1745: }

These are not thread-safe functions. What constraints exist for using these methods safely?
Update: looks like they must be called with the lock held so we should assert that.
Should also assert the count never goes negative (which I assume would be an error?).

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

> 250:     return; // JvmtiVTMSTransitionDisabler is no-op without virtual threads
> 251:   }
> 252:   if (Thread::current_or_null() == NULL) {

Nit: please use `nullptr` in new code

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

> 271:   }
> 272:   _is_SR = is_SR;
> 273:   _vthread = NULL;

Nit: should initialize in init list

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

> 296:   HandleMark hm(thread);
> 297:   Handle vth = Handle(thread, JNIHandles::resolve_external_guard(_vthread));
> 298:   if (!java_lang_VirtualThread::is_instance(vth())) {

How can this condition not be true? Should it be an assertion?

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

> 302: 
> 303:   ThreadBlockInVM tbivm(thread);
> 304:   MonitorLocker ml(JvmtiVTMSTransition_lock, Mutex::_no_safepoint_check_flag);

Aside: this pattern looks very odd. Why not just lock with the safepoint check?

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

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