On Thu, 15 Dec 2022 11:51:10 GMT, Serguei Spitsyn <[email protected]> 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
