RFR: 8330849: Add test to verify memory usage with recursive locking [v2]

[Roman Kennke]

On Mon, 22 Apr 2024 21:21:28 GMT, Leonid Mesnik <lmesnik at openjdk.org> wrote:

>> Roman Kennke has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Make test less restrictive
>
> test/hotspot/jtreg/runtime/locking/TestRecursiveMonitorChurn.java line 35:
> 
>> 33:  * @summary Tests that recursive locking doesn't cause excessive native memory usage
>> 34:  * @library /test/lib
>> 35:  * @run main TestRecursiveMonitorChurn
> 
> Better to use 
> @run driver TestRecursiveMonitorChurn
> for tests which fork VM.

Ok, did that. Can you explain to me what difference does it make?

> test/hotspot/jtreg/runtime/locking/TestRecursiveMonitorChurn.java line 39:
> 
>> 37: public class TestRecursiveMonitorChurn {
>> 38:     static class Monitor {
>> 39:         volatile int i, j;
> 
> Not sure if volatile makes sense here. But it is better to make i, j public static so the compiler can't optimize them.
> (The even better would be to print them in main()).

Right. Might be useful if test framework had something like blackholes in JMH.

> test/hotspot/jtreg/runtime/locking/TestRecursiveMonitorChurn.java line 88:
> 
>> 86:                         throw new RuntimeException("Allocated too many monitors");
>> 87:                     }
>> 88:                     foundLine = true;
> 
> Small nit.
> There is should be only one line like ' Object Monitors (reserved=20800624, committed=20800624)'
> Some might just return true here? So no foundLine is needed.

True. Fixed.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18899#discussion_r1575668188
PR Review Comment: https://git.openjdk.org/jdk/pull/18899#discussion_r1575668872
PR Review Comment: https://git.openjdk.org/jdk/pull/18899#discussion_r1575669106