On Fri, 17 Nov 2023 10:30:59 GMT, Serguei Spitsyn <sspit...@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. > > Serguei Spitsyn has updated the pull request incrementally with one > additional commit since the last revision: > > review: make new fields volatile, use Atomic for access/update Hi Serguei, One comment below. Thanks src/hotspot/share/prims/jvmtiThreadState.cpp line 427: > 425: oop vt = JNIHandles::resolve_external_guard(vthread); > 426: > 427: if (!sync_protocol_enabled()) { Isn't the check racy? So we could see that there is no disabler, but before setting anything a disabler can run and succeed. Then we would never notice it and will just return after setting these fields. I think you need to check this after setting _VTMS_transition_count and the vthread transition bit. ------------- PR Review: https://git.openjdk.org/jdk/pull/16688#pullrequestreview-1756428869 PR Review Comment: https://git.openjdk.org/jdk/pull/16688#discussion_r1410012212
