On Wed, 7 Feb 2024 22:37:55 GMT, Daniel D. Daugherty <dcu...@openjdk.org> wrote:
>> Serguei Spitsyn has updated the pull request with a new target base due to a
>> merge or a rebase. The incremental webrev excludes the unrelated changes
>> brought in by the merge/rebase. The pull request contains three additional
>> commits since the last revision:
>>
>> - Merge
>> - review: thread in notify waiter list can't be BLOCKED
>> - 8324677: Specification clarification needed for JVM TI
>> GetObjectMonitorUsage
>
> src/hotspot/share/prims/jvmtiEnvBase.cpp line 1521:
>
>> 1519: // we have contending and/or waiting threads
>> 1520: if (nWant > 0) {
>> 1521: // we have contending threads
>
> This block includes this logic:
>
> // get_pending_threads returns only java thread so we do not need to
> // check for non java threads.
> GrowableArray<JavaThread*>* wantList =
> Threads::get_pending_threads(tlh.list(), nWant, (address)mon);
> if (wantList->length() < nWant) {
> // robustness: the pending list has gotten smaller
> nWant = wantList->length();
> }
>
> `Threads::get_pending_threads()` only returns threads where the
> `current_pending_monitor` field is set for the specific monitor. That
> happens only on contended enter and does not happen on contended
> re-enter so this logic will already strip out any threads in `wait()` that
> have not been notified and have not had their wait timers expire.
> It will also strip out any waiters that have been notified or had
> their wait timeouts expire.
>
> This means even if we fix the reenter code to increment the contentions
> field, this logic will reduce that `nWant` value. Of course, the way around
> that is to also update the reenter code to properly set the
> `current_pending_monitor`
> field and then the reentering threads won't be filtered out...
Yes, I've figured this out now. Thank you for pointing to it.
It feels, the counts can be calculated correctly without touching the
implementation of `current_pending_monitor()`, `current_waiting_monitor()`,
`_contensions` and `_waiters`.
At least, I'll try to fix it locally in the
`JvmtiEnvBase::get_object_monitor_usage()`.
Please, let me know what do you prefer.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/17680#discussion_r1482749279
