RFR: 8343957: Rename ObjectMonitor::owner_from() and JavaThread::_lock_id
David Holmes
dholmes at openjdk.org
Wed Dec 4 01:47:39 UTC 2024
On Tue, 3 Dec 2024 19:10:55 GMT, Patricio Chilano Mateo <pchilanomate at 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
More information about the serviceability-dev
mailing list