RFR: 8329351: Add runtime/Monitor/TestRecursiveLocking.java for recursive Java monitor stress testing [v3]

Daniel D. Daugherty dcubed at openjdk.org
Wed Nov 20 16:30:27 UTC 2024


On Wed, 20 Nov 2024 15:43:39 GMT, Fredrik Bredberg <fbredberg at openjdk.org> wrote:

>> @dcubed-ojdk is too busy, so I was asked to take over [https://github.com/openjdk/jdk/pull/18664](https://github.com/openjdk/jdk/pull/18664) and finish it.
>> 
>> That old PR contains all the details and also got some review comments, which I have tried to address in this new PR.
>> 
>> Tests run without failures on supported platforms.
>
> Fredrik Bredberg has updated the pull request incrementally with one additional commit since the last revision:
> 
>   s/removed/removes/

There are more tweaks needed for the usage() function in order
to get correct default output values.

test/hotspot/jtreg/runtime/Monitor/TestRecursiveLocking.java line 230:

> 228:             counter++;
> 229: 
> 230:             // Legacy mode has no lock stack, I.e. there is no limit

nit typography: s/stack, I.e. there/stack, i.e., there/
- lower case "i.e." followed by a comma

test/hotspot/jtreg/runtime/Monitor/TestRecursiveLocking.java line 238:

> 236:             }
> 237: 
> 238:             // We havn't reached the stack lock capacity (recursion

nit typo: s/havn't/haven't /

test/hotspot/jtreg/runtime/Monitor/TestRecursiveLocking.java line 392:

> 390:     }
> 391: 
> 392:     static void usage(int mode, int n_secs) {

Please drop these parameters.

test/hotspot/jtreg/runtime/Monitor/TestRecursiveLocking.java line 399:

> 397:         System.err.println("where:");
> 398:         System.err.println("    n_secs  ::= > 0");
> 399:         System.err.println("            Default n_secs is " + n_secs + ".");

Please change this to:

        System.err.println("            Default n_secs is " + def_n_secs + ".");

test/hotspot/jtreg/runtime/Monitor/TestRecursiveLocking.java line 402:

> 400:         System.err.println("    mode    ::= 1 - outer and inner");
> 401:         System.err.println("            ::= 2 - alternate A and B");
> 402:         System.err.println("            Default mode is " + mode + ".");

Please change this to:

       System.err.println("            Default mode is " + def_mode + ".");

test/hotspot/jtreg/runtime/Monitor/TestRecursiveLocking.java line 408:

> 406:     public static void main(String... argv) throws Exception {
> 407:         int mode = 2;
> 408:         int n_secs = 30;

Please change this to:

        static final int def_mode = 2;
        static final int def_n_secs = 30;
        int mode = def_mode;
        int n_secs = def_n_secs;

test/hotspot/jtreg/runtime/Monitor/TestRecursiveLocking.java line 411:

> 409: 
> 410:         if (argv.length != 0 && argv.length != 1 && argv.length != 2) {
> 411:             usage(mode, n_secs);

Please drop these parameters.

test/hotspot/jtreg/runtime/Monitor/TestRecursiveLocking.java line 424:

> 422:                 System.err.println("ERROR: '" + argv[0]
> 423:                                    + "': invalid n_secs value.");
> 424:                 usage(mode, n_secs);

Please drop these parameters.

test/hotspot/jtreg/runtime/Monitor/TestRecursiveLocking.java line 439:

> 437:                     System.err.println("ERROR: '" + argv[1]
> 438:                                        + "': invalid mode value.");
> 439:                     usage(mode, n_secs);

Please drop these parameters.

The reason for adding `def_mode` and `def_n_secs` above is that when this `usage()` call
is made, it will print the current `n_secs` value instead of the actual default `n_secs` value.
It will also print the `mode` value returned by L429 instead of the actual default `mode` value.

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

Changes requested by dcubed (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/22238#pullrequestreview-2449093166
PR Review Comment: https://git.openjdk.org/jdk/pull/22238#discussion_r1850604654
PR Review Comment: https://git.openjdk.org/jdk/pull/22238#discussion_r1850605970
PR Review Comment: https://git.openjdk.org/jdk/pull/22238#discussion_r1850618037
PR Review Comment: https://git.openjdk.org/jdk/pull/22238#discussion_r1850614564
PR Review Comment: https://git.openjdk.org/jdk/pull/22238#discussion_r1850615392
PR Review Comment: https://git.openjdk.org/jdk/pull/22238#discussion_r1850613746
PR Review Comment: https://git.openjdk.org/jdk/pull/22238#discussion_r1850620329
PR Review Comment: https://git.openjdk.org/jdk/pull/22238#discussion_r1850620712
PR Review Comment: https://git.openjdk.org/jdk/pull/22238#discussion_r1850624071


More information about the hotspot-dev mailing list