RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning [v16]

[Patricio Chilano Mateo]

On Tue, 29 Oct 2024 02:56:30 GMT, Serguei Spitsyn <sspitsyn at openjdk.org> wrote:

>> Patricio Chilano Mateo has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Fix comment in VThreadWaitReenter
>
> src/hotspot/share/prims/jvmtiEnvBase.cpp line 1082:
> 
>> 1080:     } else {
>> 1081:       assert(vthread != nullptr, "no vthread oop");
>> 1082:       oop oopCont = java_lang_VirtualThread::continuation(vthread);
> 
> Nit: The name `oopCont` does not match the HotSpot naming convention.
>        What about `cont_oop` or even better just `cont` as at the line 2550?

Renamed to cont.

> src/hotspot/share/prims/jvmtiExport.cpp line 1682:
> 
>> 1680: 
>> 1681:   // On preemption JVMTI state rebinding has already happened so get it always directly from the oop.
>> 1682:   JvmtiThreadState *state = java_lang_Thread::jvmti_thread_state(JNIHandles::resolve(vthread));
> 
> I'm not sure this change is right. The `get_jvmti_thread_state()` has a role to lazily create a `JvmtiThreadState` if it was not created before. With this change the `JvmtiThreadState` creation can be missed if the `unmount` event is the first event encountered for this particular virtual thread. You probably remember that lazy creation of  the `JvmtiThreadState`'s is an important optimization to avoid big performance overhead when a JVMTI agent is present.

Right, good find. I missed `get_jvmti_thread_state ` will also create the state if null. How about this fix: https://github.com/pchilano/jdk/commit/baf30d92f79cc084824b207a199672f5b7f9be88
I now also see that JvmtiVirtualThreadEventMark tries to save some state of the JvmtiThreadState for the current thread before the callback, which is not the JvmtiThreadState of the vthread for this case. Don't know if something needs to change there too.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1823319745
PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1823322449