RFR: 8247972: incorrect implementation of JVM TI GetObjectMonitorUsage [v21]

[Serguei Spitsyn]

On Mon, 4 Mar 2024 03:07:58 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> Serguei Spitsyn has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   rename after merge: jvmti_common.h to jvmti_common.hpp
>
> src/hotspot/share/prims/jvmtiEnvBase.cpp line 1489:
> 
>> 1487:     assert(mon != nullptr, "must have monitor");
>> 1488:     // this object has a heavyweight monitor
>> 1489:     nWant = mon->contentions(); // # of threads contending for monitor
> 
> Please clarify comment as
> 
> // # of threads contending for monitor entry, but not re-entry
> 
> (Fixed typos - sorry)

Will fix, thanks.

> src/hotspot/share/prims/jvmtiEnvBase.cpp line 1490:
> 
>> 1488:     // this object has a heavyweight monitor
>> 1489:     nWant = mon->contentions(); // # of threads contending for monitor
>> 1490:     nWait = mon->waiters();     // # of threads in Object.wait()
> 
> Please clarify the comment as
> 
> // # of threads waiting for notification, or to re-enter monitor, in Object.wait()

Good catch, thanks. Will fix.

> src/hotspot/share/prims/jvmtiEnvBase.cpp line 1491:
> 
>> 1489:     nWant = mon->contentions(); // # of threads contending for monitor
>> 1490:     nWait = mon->waiters();     // # of threads in Object.wait()
>> 1491:     wantList =  Threads::get_pending_threads(tlh.list(), nWant + nWait, (address)mon);
> 
> Please add a comment
> 
> // Get the actual set of threads trying to enter, or re-enter, the monitor.

Will add, thanks.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/17680#discussion_r1510895824
PR Review Comment: https://git.openjdk.org/jdk/pull/17680#discussion_r1510894066
PR Review Comment: https://git.openjdk.org/jdk/pull/17680#discussion_r1510896692