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

[Patricio Chilano Mateo]

On Tue, 29 Oct 2024 20:39:44 GMT, Dean Long <dlong at openjdk.org> wrote:

>> Patricio Chilano Mateo has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Improve comment in SharedRuntime::generate_native_wrapper
>
> src/hotspot/share/code/nmethod.cpp line 712:
> 
>> 710:   JavaThread* thread = reg_map->thread();
>> 711:   if ((thread->has_last_Java_frame() && fr.sp() == thread->last_Java_sp())
>> 712:       JVMTI_ONLY(|| (method()->is_continuation_enter_intrinsic() && thread->on_monitor_waited_event()))) {
> 
> I'm guessing this is because JVMTI can cause a safepoint?  This might need a comment.

I added a comment already in `vthread_monitor_waited_event()` in ObjectMonitor.cpp. I think it's better placed there.

> src/hotspot/share/code/nmethod.cpp line 1302:
> 
>> 1300:     _compiler_type           = type;
>> 1301:     _orig_pc_offset          = 0;
>> 1302:     _num_stack_arg_slots     = 0;
> 
> Was the old value wrong, unneeded, or is this set somewhere else?  If this field is not used, then we might want to set it to an illegal value in debug builds.

We read this value from the freeze/thaw code in several places. Since the only compiled native frame we allow to freeze is Object.wait0 the old value would be zero too. But I think the correct thing is to just set it to zero always since a value > 0 is only meaningful for Java methods.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1821594779
PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1821595264