On Tue, 20 Jan 2026 00:25:18 GMT, David Holmes <[email protected]> wrote:
>> Anton Artemov has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> 8366659: Addressed reviewers' comments, added comments, renamed tests.
>
> src/hotspot/share/runtime/objectMonitor.cpp line 2055:
>
>> 2053: // This is is now conditional as if the monitor_waited even
>> 2054: // is allowed, then a thread, even virtual, should not be
>> moved to
>> 2055: // the entry_list, but rather unparked and let run. See the
>> comment below.
>
> Suggestion:
>
> // If we will add the vthread to the entry list below then we need to
> // increment the counter *before* doing so.
> // Adding to _entry_list uses Atomic::cmpxchg() which already provides
> // a fence that prevents reordering of the stores.
> if (!JvmtiExport::should_post_monitor_waited()) {
Addressed.
> src/hotspot/share/runtime/objectMonitor.cpp line 2066:
>
>> 2064: // If the monitor_waited JVMTI event is not allowed, a thread is
>> 2065: // transferred to the entry_list, and it will eventually be
>> unparked
>> 2066: // only when it is chosen to become a successor.
>
> Suggestion:
>
> // only when it is chosen to become the successor.
Thanks for spotting, addressed.
> src/hotspot/share/runtime/objectMonitor.cpp line 2077:
>
>> 2075: // avoid a problem of having a suspension point when posting
>> 2076: // the monitor_waited JVMTI event, as suspending such a thread
>> 2077: // is no harm.
>
> Suggestion:
>
> // However, if the monitor_waited event is allowed, then
> // the thread is set to state TS_RUN and unparked. The thread
> // will then contend directly to reacquire the monitor and
> // avoids being flagged as the successor. This avoids the problem
> // of having a thread suspend whilst it is the successor.
Addressed.
> src/hotspot/share/runtime/objectMonitor.cpp line 2103:
>
>> 2101: evt->unpark();
>> 2102: }
>> 2103: else if (java_lang_VirtualThread::set_onWaitingList(vthread,
>> vthread_list_head())) {
>
> Suggestion:
>
> } else if (java_lang_VirtualThread::set_onWaitingList(vthread,
> vthread_list_head())) {
Fixed in the lates commit.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/27040#discussion_r2707568773
PR Review Comment: https://git.openjdk.org/jdk/pull/27040#discussion_r2707568292
PR Review Comment: https://git.openjdk.org/jdk/pull/27040#discussion_r2707567423
PR Review Comment: https://git.openjdk.org/jdk/pull/27040#discussion_r2707566675
