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

[Serguei Spitsyn]

On Fri, 17 Nov 2023 05:43:21 GMT, Serguei Spitsyn <sspitsyn at openjdk.org> wrote:

>> src/hotspot/share/prims/jvmtiThreadState.cpp line 531:
>> 
>>> 529: 
>>> 530:   // JvmtiThreadState objects for virtual thread filtered events enabled globally
>>> 531:   // must be created eagerly if the interp_only_mode is enabled. Otherwise,
>> 
>> This sentence is hard to read. How about:
>> 
>> "If interp_only_mode is enabled then we must eagerly create JvmtiThreadState objects for globally enabled virtual thread filtered events."
>
> Okay, thanks. The suggestion is taken.

Fixed.

>> src/hotspot/share/prims/jvmtiThreadState.cpp line 579:
>> 
>>> 577:     VTMS_mount_end(vthread);
>>> 578:     if (JvmtiExport::can_support_virtual_threads() &&
>>> 579:         JvmtiExport::should_post_vthread_mount()) {
>> 
>> It seems odd that "can_support" can be false when "should_post" is true. I would think that "should_post" would always be false when "can_support" is false, and therefore there would be no need to check "can_support".
>
> Right, thanks. It is why this check was missed in the first place. Will undo this change.

Fixed.

>> 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; // needed for optimization
>> 
>> Say what the flag represents, not why we have it.
>
> Thank you for looking at this PR!
> Okay, thanks. Will do.

Fixed.

>> src/hotspot/share/prims/jvmtiThreadState.hpp line 257:
>> 
>>> 255:   // JvmtiThreadState objects for virtual thread filtered events enabled globally
>>> 256:   // must be created eagerly if the interp_only_mode is enabled. Otherwise,
>>> 257:   // it is an important optimization to create JvmtiThreadState objects lazily.
>> 
>> No need for this comment here. It is already at the call site, which is where it belongs. Instead the comment here should say what this API does (return true if any thread has entered interp_only_mode at any point during the JVMs execution).
>
> Thanks, good suggestion.

Fixed.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16686#discussion_r1396798836
PR Review Comment: https://git.openjdk.org/jdk/pull/16686#discussion_r1396801201
PR Review Comment: https://git.openjdk.org/jdk/pull/16686#discussion_r1396798590
PR Review Comment: https://git.openjdk.org/jdk/pull/16686#discussion_r1396799894