RFR: 8330969: scalability issue with loaded JVMTI agent [v2]

[Chris Plummer]

On Fri, 26 Apr 2024 07:45:50 GMT, Serguei Spitsyn <sspitsyn at openjdk.org> wrote:

>> This is a fix of the following JVMTI scalability issue. A closed benchmark with millions of virtual threads shows 3X-4X overhead when a JVMTI agent has been loaded. For instance, this is observable when an app is executed under control of the Oracle Studio `collect` utility.
>> For performance analysis, experiments and numbers, please, see the comment below this description.
>> 
>> The fix is to replace the global counter `_VTMS_transition_count` with the mark bit `_VTMS_transition_mark` in each `JavaThread`'.
>> 
>> Testing:
>>  - Tested with mach5 tiers 1-6
>
> Serguei Spitsyn has updated the pull request incrementally with one additional commit since the last revision:
> 
>   review: fixed minor issues: renamed function, corrected comment, removed typo in assert

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

> 364:           attempts--;
> 365:         }
> 366:         DEBUG_ONLY(if (attempts == 0) break;)

Previously `_VTMS_transition_count` considered all threads at the same time. Now you are iterating through the threads and looking at a flag in each one. Is it guaranteed that once the `_VTMS_transition_mark` flag has been verified not to be set in a thread it won't get set while still iterating in the threads loop?

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

> 431:   // Avoid using MonitorLocker on performance critical path, use
> 432:   // two-level synchronization with lock-free operations on counters.
> 433:   assert(!thread->VTMS_transition_mark(), "sanity check");

The "counters" comment needs to be updated.

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

> 454:     // Slow path: undo unsuccessful optimistic counter incrementation.
> 455:     // It can cause an extra waiting cycle for VTMS transition disablers.
> 456:     thread->set_VTMS_transition_mark(false);

The "optimistic counter incrementation" comment needs updating.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18937#discussion_r1581460754
PR Review Comment: https://git.openjdk.org/jdk/pull/18937#discussion_r1581463641
PR Review Comment: https://git.openjdk.org/jdk/pull/18937#discussion_r1581462878