On Wed, 22 Mar 2023 21:36:57 GMT, Chris Plummer <cjplum...@openjdk.org> wrote:
>> Serguei Spitsyn has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> minor cleanup in enable_virtual_threads_notify_jvmti()
>
> src/hotspot/share/prims/jvmtiExport.cpp line 389:
>
>> 387: } else {
>> 388: JvmtiVTMSTransitionDisabler::set_VTMS_notify_jvmti_events(true);
>> 389: }
>
> One thing that is a little bit confusing about the changes here is that this
> is where the fix for
> [JDK-8296324](https://bugs.openjdk.org/browse/JDK-8296324) went, but for some
> reason the fix for
> [JDK-8304303](https://bugs.openjdk.org/browse/JDK-8304303), which was pushed
> a couple of days ago, removed the
> [JDK-8296324](https://bugs.openjdk.org/browse/JDK-8296324) changes even
> though these two bugs don't seem related. The code added by
> [JDK-8296324](https://bugs.openjdk.org/browse/JDK-8296324) was:
>
> ``` if (Continuations::enabled()) {
> // Virtual threads support. There is a performance impact when VTMS
> transitions are enabled.
> java_lang_VirtualThread::set_notify_jvmti_events(true);
> + if (JvmtiEnv::get_phase() == JVMTI_PHASE_LIVE) {
> + ThreadInVMfromNative __tiv(JavaThread::current());
> + java_lang_VirtualThread::init_static_notify_jvmti_events();
> + }
> }
I agree, it is confusing.
This fragment was removed by JDK-8304303 because the function
`java_lang_VirtualThread::init_static_notify_jvmti_events()` was being removed.
It did not work correctly in the first place. It is why the JDK-8297286 was
filed. Also, it was on my plan to fix JDK-8297286 soon.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/13133#discussion_r1145449155
