On Fri, 25 Oct 2024 03:51:08 GMT, David Holmes <dhol...@openjdk.org> wrote:
>> Patricio Chilano Mateo has updated the pull request incrementally with four
>> additional commits since the last revision:
>>
>> - Rename set/has_owner_anonymous to set/has_anonymous_owner
>> - Fix comments in javaThread.hpp and Thread.java
>> - Rename nonce/nounce to seqNo in VirtualThread class
>> - Remove ObjectMonitor::set_owner_from_BasicLock()
>
> src/hotspot/share/runtime/objectMonitor.hpp line 174:
>
>> 172:
>> 173: int64_t volatile _owner; // Either tid of owner, NO_OWNER,
>> ANONYMOUS_OWNER or DEFLATER_MARKER.
>> 174: volatile uint64_t _previous_owner_tid; // thread id of the previous
>> owner of the monitor
>
> Looks odd to have the current owner as `int64_t` but we save the previous
> owner as `uint64_t`. ??
I was wondering what this was too but the _previous_owner_tid is the os thread
id, not the Java thread id.
$ grep -r JFR_THREAD_ID
jfr/support/jfrThreadId.hpp:#define JFR_THREAD_ID(thread)
(JfrThreadLocal::external_thread_id(thread))
jfr/support/jfrThreadId.hpp:#define JFR_THREAD_ID(thread)
((traceid)(thread)->osthread()->thread_id())
runtime/objectMonitor.cpp: _previous_owner_tid = JFR_THREAD_ID(current);
runtime/objectMonitor.cpp: iterator->_notifier_tid = JFR_THREAD_ID(current);
runtime/vmThread.cpp: event->set_caller(JFR_THREAD_ID(op->calling_thread()));
> src/hotspot/share/runtime/synchronizer.cpp line 1440:
>
>> 1438: }
>> 1439:
>> 1440: ObjectMonitor* ObjectSynchronizer::inflate_impl(JavaThread*
>> inflating_thread, oop object, const InflateCause cause) {
>
> `inflating_thread` doesn't sound right as it is always the current thread
> that is doing the inflating. The passed in thread may be a different thread
> trying to acquire the monitor ... perhaps `contending_thread`?
If it's always the current thread, then it should be called 'current' imo.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1816550112
PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1816551794
