On Thu, 16 Nov 2023 17:05:17 GMT, Leonid Mesnik <lmes...@openjdk.org> wrote:
>> This is an update for a performance/scalability enhancement. >> >> The `JvmtiVTMSTransitionDisabler`sync protocol is on a performance critical >> path of the virtual threads mount state transitions (VTMS transitions). It >> has a noticeable performance overhead (about 10%) which contributes to the >> combined JVMTI performance overhead when Java apps are executed with loaded >> JVMTI agents. >> >> Please, also see another/related performance issue which contributes around >> 70% of total performance overhead: >> [8308614](https://bugs.openjdk.org/browse/JDK-8308614): Enabling JVMTI >> ClassLoad event slows down vthread creation by factor 10 >> >> Testing: >> - Ran mach5 tiers 1-6 with no regressions noticed. > > src/hotspot/share/prims/jvmtiThreadState.cpp line 430: > >> 428: assert(!thread->is_in_VTMS_transition(), "VTMS_transition sanity >> check"); >> 429: thread->set_is_in_VTMS_transition(true); >> 430: java_lang_Thread::set_is_in_VTMS_transition(vt, true); > > indentation is incorrect. Thank you. Fixed now. > src/hotspot/share/prims/jvmtiThreadState.hpp line 86: > >> 84: static volatile bool _SR_mode; // there is an >> active suspender or resumer >> 85: static volatile int _VTMS_transition_count; // current >> number of VTMS transitions >> 86: static int _sync_protocol_enabled_count; // current >> number of JvmtiVTMSTransitionDisablers enabled sync protocol > > The _sync_protocol_enabled_count and _sync_protocol_enabled_permanently are > read/updated in different threads. How access to them is protected from > racing? Might be make sense to add this info in comment? Good catch, thanks. My initial intention was to make them volatile with Atomic load/store/update. Fixed now. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/16688#discussion_r1396191582 PR Review Comment: https://git.openjdk.org/jdk/pull/16688#discussion_r1396194416
