RFR: 8320239: add dynamic switch for JvmtiVTMSTransitionDisabler sync protocol [v2]

[Patricio Chilano Mateo]

On Fri, 17 Nov 2023 10:30:59 GMT, Serguei Spitsyn <sspitsyn at 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