On Wed, 21 Jan 2026 04:45:49 GMT, Patricio Chilano Mateo
<[email protected]> wrote:
>> Anton Artemov has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> 8366659: Fixed whitespace.
>
> src/hotspot/share/runtime/objectMonitor.cpp line 558:
>
>> 556: bool is_virtual = ce != nullptr && ce->is_virtual_thread();
>> 557: if (is_virtual) {
>> 558: notify_contended_enter(current, post_jvmti_events);
>
> We should also add `post_jvmti_event &&` to L567 below.
Good catch, added!
> src/hotspot/share/runtime/objectMonitor.cpp line 2079:
>
>> 2077: iterator->TState = ObjectWaiter::TS_RUN;
>> 2078:
>> 2079: OrderAccess::fence();
>
> Is the fence needed? We already synchronize with `_wait_set_lock`.
Actually it is not, as we are inside of a wider critical section.
> src/hotspot/share/runtime/objectMonitor.cpp line 2105:
>
>> 2103: }
>> 2104: return did_notify;
>> 2105: }
>
> Given that there are more differences than common code, I think it's probably
> better to just separate the vthread and platform thread paths to make it
> easier to read. Something like this:
> https://github.com/openjdk/jdk/compare/pr/27040...pchilano:jdk:altnotify
> You could add a comment on top of the method about the behavior when
> monitor_waited event is enabled.
Thanks, I have incorporated your changes.
> src/hotspot/share/runtime/objectMonitor.cpp line 2291:
>
>> 2289: // because, if monitor_waited JVMTI event is allowed, there can be a
>> vthead which
>> 2290: // is not on the entry_list, but has been notified in the sense that
>> it has been
>> 2291: // unparked directly in notify_internal(). Its state is then TS_RUN.
>
> Suggestion for shorter comment:
>
> // We check the state rather than was_notified because, when JVMTI
> // monitor_waited event is enabled, the notifier only unparks the waiter
> // without adding it to the entry_list.
Yes, a bit wordy, fixed.
> src/hotspot/share/runtime/objectMonitor.hpp line 367:
>
>> 365: bool enter_is_async_deflating();
>> 366: void notify_contended_enter(JavaThread *current, bool
>> post_jvmti_events = true);
>> 367: void post_waited_event(JavaThread* current,
>> EventJavaMonitorWait* wait_event, bool was_notified, ObjectWaiter* node,
>> jlong millis, int ret);
>
> Leftover?
Yes, a leftover from a previous iteration, thanks for spotting.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/27040#discussion_r2712553017
PR Review Comment: https://git.openjdk.org/jdk/pull/27040#discussion_r2712554279
PR Review Comment: https://git.openjdk.org/jdk/pull/27040#discussion_r2712557326
PR Review Comment: https://git.openjdk.org/jdk/pull/27040#discussion_r2712553670
PR Review Comment: https://git.openjdk.org/jdk/pull/27040#discussion_r2712553472
