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

[Fredrik Bredberg]

On Tue, 19 Nov 2024 20:33:08 GMT, Daniel D. Daugherty <dcubed at openjdk.org> wrote:

>> Fredrik Bredberg has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Update after review
>
> 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 grammar/typography: s/. I.e. there/, i.e., there

fixed

> test/hotspot/jtreg/runtime/Monitor/TestRecursiveLocking.java line 238:
> 
>> 236:             }
>> 237: 
>> 238:             // We havn't reached the stack lock capasity (recursion
> 
> nit typo: s/capasity/capacity/

fixed

> test/hotspot/jtreg/runtime/Monitor/TestRecursiveLocking.java line 270:
> 
>> 268:         }
>> 269: 
>> 270:         // This test nests x recurcive locks of INNER, in x recursive
> 
> nit typo: s/recurcive/recursive/

fixed

> test/hotspot/jtreg/runtime/Monitor/TestRecursiveLocking.java line 271:
> 
>> 269: 
>> 270:         // This test nests x recurcive locks of INNER, in x recursive
>> 271:         // locks of OUTER.  The number x is taken from the max number
> 
> nit spacing: s/OUTER.  The/OUTER. The/

fixed

> test/hotspot/jtreg/runtime/Monitor/TestRecursiveLocking.java line 316:
> 
>> 314:                 } else {
>> 315:                     // Second time we want to lock A, the lock stack
>> 316:                     // looks like this [A, B].  Lightweight locking
> 
> nit spacing: s/B].  Lightweight/B]. Lightweight/

fixed

> test/hotspot/jtreg/runtime/Monitor/TestRecursiveLocking.java line 318:
> 
>> 316:                     // looks like this [A, B].  Lightweight locking
>> 317:                     // doesn't allow interleaving ([A, B, A]), instead
>> 318:                     // it inflates A and removed it from the lock
> 
> nit typo: s/removed/removes/

fixed

> test/hotspot/jtreg/runtime/Monitor/TestRecursiveLocking.java line 375:
> 
>> 373:             }
>> 374:             // Implied else: for LM_MONITOR or LM_LIGHTWEIGHT it can be
>> 375:             // either inflated or not point because A is not locked anymore
> 
> nit typo: s/either inflated or not point because/either inflated or not because/

fixed

> 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 15.");
> 
> Based on L408 below, it looks like default n_secs is now `30`.

fixed

> 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 1.");
> 
> Based on L407 below, it looks like default mode is now `2`.

fixed

> test/hotspot/jtreg/runtime/Monitor/TestRecursiveLocking.java line 503:
> 
>> 501:                     waiter.wait();
>> 502:                 } catch (InterruptedException ie) {
>> 503:                 }
> 
> Do you want to add this:
> 
>             // This should not happen.
>             ie.printStackTrace();
> 
> here also?

fixed

> test/hotspot/jtreg/runtime/Monitor/TestRecursiveLocking.java line 541:
> 
>> 539:                     waiter.wait();
>> 540:                 } catch (InterruptedException ie) {
>> 541:                 }
> 
> Do you want to add this:
> 
>             // This should not happen.
>             ie.printStackTrace();
> 
> here also?

fixed

> test/hotspot/jtreg/runtime/Monitor/TestRecursiveLocking.java line 556:
> 
>> 554:                 waiter.wait();
>> 555:             } catch (InterruptedException ie) {
>> 556:             }
> 
> Do you want to add this:
> 
>             // This should not happen.
>             ie.printStackTrace();
> 
> here also?

fixed

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

PR Review Comment: https://git.openjdk.org/jdk/pull/22238#discussion_r1850335375
PR Review Comment: https://git.openjdk.org/jdk/pull/22238#discussion_r1850334888
PR Review Comment: https://git.openjdk.org/jdk/pull/22238#discussion_r1850336106
PR Review Comment: https://git.openjdk.org/jdk/pull/22238#discussion_r1850335585
PR Review Comment: https://git.openjdk.org/jdk/pull/22238#discussion_r1850336354
PR Review Comment: https://git.openjdk.org/jdk/pull/22238#discussion_r1850340633
PR Review Comment: https://git.openjdk.org/jdk/pull/22238#discussion_r1850338178
PR Review Comment: https://git.openjdk.org/jdk/pull/22238#discussion_r1850338548
PR Review Comment: https://git.openjdk.org/jdk/pull/22238#discussion_r1850338932
PR Review Comment: https://git.openjdk.org/jdk/pull/22238#discussion_r1850339155
PR Review Comment: https://git.openjdk.org/jdk/pull/22238#discussion_r1850339678
PR Review Comment: https://git.openjdk.org/jdk/pull/22238#discussion_r1850339867