RFR: 8308614: Enabling JVMTI ClassLoad event slows down vthread creation by factor 10 [v4]

[Daniel D. Daugherty]

On Thu, 30 Nov 2023 02:08:39 GMT, Serguei Spitsyn <sspitsyn at openjdk.org> wrote:

>> This is a fix of a performance/scalability related issue. The `JvmtiThreadState` objects for virtual thread filtered events enabled globally are created eagerly because it is needed when the `interp_only_mode` is enabled. Otherwise, some events which are generated in `interp_only_mode` from the debugging version of interpreter chunks can be missed.
>> However, it has to be okay to avoid eager creation of these object if no `interp_only_mode` has ever been requested.
>> It seems to be an extremely important optimization to create JvmtiThreadState objects lazily in such cases.
>> It is done by introducing the flag `JvmtiThreadState::_seen_interp_only_mode` which indicates when the `JvmtiThreadState` objects have to be created eagerly.
>> 
>> Additionally, the fix includes the following related changes:
>>  - Use condition double checking idiom for `MutexLocker mu(JvmtiThreadState_lock)` in the function `JvmtiVTMSTransitionDisabler::VTMS_mount_end` which is on a performance-critical path and looks like this:
>> 
>>   JvmtiThreadState* state = thread->jvmti_thread_state();
>>   if (state != nullptr && state->is_pending_interp_only_mode()) {
>>     MutexLocker mu(JvmtiThreadState_lock);
>>     state = thread->jvmti_thread_state();
>>     if (state != nullptr && state->is_pending_interp_only_mode()) {
>>       JvmtiEventController::enter_interp_only_mode();
>>     }
>>   }
>> 
>> 
>>  - Add extra check of `JvmtiExport::can_support_virtual_threads()` when virtual thread mount and unmount are posted.
>>  - Minor: Added a `ThreadsListHandle` to the `JvmtiEventControllerPrivate::enter_interp_only_mode`. It is needed because of the dynamic creation of compensating carrier threads which is racy for JVMTI `SetEventNotificationMode` implementation.
>> 
>> Performance mesurements:
>> - Without this fix the test provided by the bug submitter gives execution numbers:
>>   - no ClassLoad events enabled:     3251 ms
>>   - ClassLoad events are enabled: 40534 ms
>>   
>> - With the fix:
>>   - no ClassLoad events enabled:    3270 ms
>>   - ClassLoad events are enabled:   3385 ms
>>   
>> Testing:
>>  - Ran mach5 tiers 1-6, no regressions are noticed
>
> Serguei Spitsyn has updated the pull request incrementally with one additional commit since the last revision:
> 
>   review: remove newly added ThreadsListHandle from enter_interp_only_mode

Changes requested by dcubed (Reviewer).

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

> 528:   assert(!thread->is_in_tmp_VTMS_transition(), "sanity check");
> 529: 
> 530:   // If interp_only_mode is enabled then we must eagerly create JvmtiThreadState

typo: s/is enabled/has been enabled/

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

> 534:     JvmtiEventController::thread_started(thread);
> 535:   }
> 536:   if (JvmtiExport::should_post_vthread_start()) {

Why is this check: `if (JvmtiExport::can_support_virtual_threads()) {` removed?

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

> 550:     JvmtiExport::post_vthread_unmount(vthread);
> 551:   }
> 552:   if (JvmtiExport::can_support_virtual_threads()) {

Why is this check: if (JvmtiExport::can_support_virtual_threads()) { removed?

src/hotspot/share/prims/jvmtiThreadState.hpp line 234:

> 232:   inline void set_head_env_thread_state(JvmtiEnvThreadState* ets);
> 233: 
> 234:   static bool _seen_interp_only_mode; // interp_only_mode was requested once

perhaps: s/requested once/requested at least once/

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

PR Review: https://git.openjdk.org/jdk/pull/16686#pullrequestreview-1758088444
PR Review Comment: https://git.openjdk.org/jdk/pull/16686#discussion_r1411066065
PR Review Comment: https://git.openjdk.org/jdk/pull/16686#discussion_r1411051100
PR Review Comment: https://git.openjdk.org/jdk/pull/16686#discussion_r1411051508
PR Review Comment: https://git.openjdk.org/jdk/pull/16686#discussion_r1411059699