On Tue, 3 Dec 2024 19:10:55 GMT, Patricio Chilano Mateo
<pchilanom...@openjdk.org> wrote:
> Please review this small renaming patch. During the review of JDK-8338383
> there were some comments about improving the naming for
> `ObjectMonitor::owner_from()` and `JavaThread::_lock_id`. These originate
> from the changes introduced to inflated monitors, where we now record the
> `java.lang.Thread.tid` of the owner in the ObjectMonitor's `_owner` field
> instead of a `JavaThread*`. I renamed `_lock_id` as `_monitor_owner_id` and
> `owner_from()` as `owner_id_from()`.
>
> Thanks,
> Patricio
Thanks for making the changes.
One minor nit, but looks good.
src/hotspot/share/runtime/javaThread.hpp line 174:
> 172: void set_monitor_owner_id(int64_t val) {
> 173: assert(val >= ThreadIdentifier::initial() && val <
> ThreadIdentifier::current(), "");
> 174: _monitor_owner_id = val;
Nit: Using `id` rather than `val` would be more consistent with other changes
(`ObjectMonitor::owner_id_from`)
src/hotspot/share/runtime/threads.cpp line 1363:
> 1361: p->print_stack_on(st);
> 1362: if (p->is_vthread_mounted()) {
> 1363: st->print_cr(" Mounted virtual thread #" INT64_FORMAT,
> java_lang_Thread::thread_id(p->vthread()));
Was initially unsure why `p->lock_id()` didn't change to
`p->monitor_owner_id()`, but here you want the thread-id not something that
happens to match the thread-id.
-------------
Marked as reviewed by dholmes (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/22524#pullrequestreview-2477079004
PR Review Comment: https://git.openjdk.org/jdk/pull/22524#discussion_r1868577497
PR Review Comment: https://git.openjdk.org/jdk/pull/22524#discussion_r1868580851
