On Fri, 26 Apr 2024 07:45:50 GMT, Serguei Spitsyn <sspit...@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
