On Tue, 29 Oct 2024 20:39:44 GMT, Dean Long <[email protected]> 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
