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