On Thu, 25 Apr 2024 20:39:03 GMT, Alex Menkov <amen...@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
>
> src/hotspot/share/prims/jvmtiEnvBase.cpp line 1638:
>
>> 1636: // Iterates over all JavaThread's, counts VTMS transitions and
>> restores
>> 1637: // jt->jvmti_thread_state() and jt->jvmti_vthread() for VTMS
>> transition protocol.
>> 1638: void count_transitions_and_correct_jvmti_thread_states() {
>
> The method doesn't count anything anymore.
> Rename it to `correct_jvmti_thread_states()`?
> Comment needs to be updated too.
Good suggestion, thanks. Renamed function and corrected the comment.
> src/hotspot/share/prims/jvmtiThreadState.cpp line 501:
>
>> 499: oop vt = JNIHandles::resolve_external_guard(vthread);
>> 500: java_lang_Thread::set_is_in_VTMS_transition(vt, false);
>> 501: assert(thread->VTMS_transition_mark(), "sanity ed_heck");
>
> Suggestion:
>
> assert(thread->VTMS_transition_mark(), "sanity check");
Thanks. Fixed now.
> src/hotspot/share/runtime/javaThread.hpp line 668:
>
>> 666: void toggle_is_disable_suspend() { _is_disable_suspend
>> = !_is_disable_suspend; };
>> 667:
>> 668: bool VTMS_transition_mark() { return
>> Atomic::load(&_VTMS_transition_mark); }
>
> Suggestion:
>
> bool VTMS_transition_mark() const { return
> Atomic::load(&_VTMS_transition_mark); }
Good suggestion, thanks. Fixed now.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/18937#discussion_r1580607373
PR Review Comment: https://git.openjdk.org/jdk/pull/18937#discussion_r1580610256
PR Review Comment: https://git.openjdk.org/jdk/pull/18937#discussion_r1580612160
