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

[David Holmes]

On Tue, 13 Feb 2024 08:34:57 GMT, Serguei Spitsyn <sspitsyn at openjdk.org> wrote:

>> test/hotspot/jtreg/vmTestbase/nsk/jvmti/GetObjectMonitorUsage/objmonusage003.java line 33:
>> 
>>> 31:     final static int NUMBER_OF_ENTERER_THREADS = 4;
>>> 32:     final static int NUMBER_OF_WAITER_THREADS  = 4;
>>> 33:     final static int NUMBER_OF_THREADS = NUMBER_OF_ENTERER_THREADS + NUMBER_OF_WAITER_THREADS;
>> 
>> This testing is not sufficient. We have three states of interest:
>> - entering the monitor directly
>> - waiting in the monitor via Object.wait
>> - re-rentering the monitor after being notified (or timed-out or interrupted)
>> - we need to check all of these conditions in permutations covering zero and non-zero for each one, and then see that we get the correct counts back from JVM TI.
>
> Thank you for the comment, David.
> Now the test checks:
> - the threads waiting to enter the monitor are returned correctly with all permutations of threads waiting to be notified and threads waiting to re-enter the monitor
> Initially, there are 4 threads waiting to be notified and zero threads waiting to re-enter the monitor.
> They are notified one by with the subsequent checks, so all the pairs are checked `<re-enter,wait-for-notif>`: `<0,4>, <1,3>, <2,2>, <3,1>, <4,0>`
> 
> I'm not sure why do you think all permutations covering zero and non-zero for each one are needed.
> At least, it looks like an overkill to me. But if you still think it is really important then I can add cases with zero threads waiting to enter the monitor with the same permutations of threads waiting to be notified and threads re-entering the monitor.

If you don't check all the zero/non-zero permutations for the three counts of interest then you don't know that you are combining them the right way to give the two counts reported by the API.
Note that checking "all the pairs" is not really necessary as all non-zero values fall in the same equivalence class for testing purposes.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17680#discussion_r1488795667