jdk icon indicating copy to clipboard operation
jdk copied to clipboard

8329351: add runtime/Monitor/TestRecursiveLocking.java for recursive Java monitor stress testing

Open dcubed-ojdk opened this issue 10 months ago • 11 comments

The first stress test to be added is:

runtime/Monitor/TestRecursiveLocking.java

It has been tested with a couple of 12 hour runs on my MBP13:

  • 3 build configs: release, fastdebug, slowdebug, 4 hours each, total 12 hours
  • 18 test configs, 800 seconds each, total 4 hours

The default configuration has been tested in Mach5 Tier2 and the StressWrapper_TestRecursiveLocking_36M.java version has been tested in Mach5 Tier3.


Progress

  • [ ] Change must be properly reviewed (1 review required, with at least 1 Reviewer)
  • [x] Change must not contain extraneous whitespace
  • [x] Commit message must refer to an issue

Warning

 ⚠️ Found leading lowercase letter in issue title for 8329351: add runtime/Monitor/TestRecursiveLocking.java for recursive Java monitor stress testing

Issue

  • JDK-8329351: add runtime/Monitor/TestRecursiveLocking.java for recursive Java monitor stress testing (Enhancement - P4)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/18664/head:pull/18664
$ git checkout pull/18664

Update a local copy of the PR:
$ git checkout pull/18664
$ git pull https://git.openjdk.org/jdk.git pull/18664/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 18664

View PR using the GUI difftool:
$ git pr show -t 18664

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/18664.diff

Webrev

Link to Webrev Comment

dcubed-ojdk avatar Apr 05 '24 19:04 dcubed-ojdk

Gory details about the test:

runtime/Monitor/TestRecursiveLocking.java

  • This test has two modes:
    • 1 - outer and inner
    • 2 - alternate A and B

The SynchronizedObject class and the OUTER followed by INNER testing model is adapted from [~aboldtch]'s test program: runtime/lockStack/TestLockStackCapacity.java

There are a total of 18 configs for this test:

  • 6 groups of the three LockingMode configs:
    • 0 - LM_MONITOR
    • 1 - LM_LEGACY
    • 2 - LM_LIGHTWEIGHT
  • The first group tests recursive locking in -Xint in outer then inner mode.
  • The second group tests recursive locking in -Xint in alternate A and B mode.
  • The third group tests recursive locking in C1 in outer then inner mode.
  • The fourth group tests recursive locking in C1 in alternate A and B mode.
  • The fifth group tests recursive locking in C2 in outer then inner mode.
  • The sixth group tests recursive locking in C2 in alternate A and B mode.

By default, each of the 18 testing configs runs for 5 seconds for a total of 90 seconds of execution time with JOBS=1. Obviously higher JOBS values will do things in parallel as resources are available. The default version of the test is NOT run in Tier1; it is run in Tier2 intentionally since this is a stress test.

runtime/Monitor/StressWrapper_TestRecursiveLocking_36M.java is a wrapper that executes runtime/Monitor/TestRecursiveLocking.java for 2 minutes for each of the 18 configs for a total of 36 minutes of execution time with JOBS=1. Again, higher JOBS values will do things in parallel as resources are available. The StressWrapper_TestRecursiveLocking_36M.java version of the test is run in Tier3 and is NOT run in Tier1 or Tier2.

dcubed-ojdk avatar Apr 05 '24 19:04 dcubed-ojdk

/label add hotspot-runtime

dcubed-ojdk avatar Apr 05 '24 19:04 dcubed-ojdk

:wave: Welcome back dcubed! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

bridgekeeper[bot] avatar Apr 05 '24 19:04 bridgekeeper[bot]

❗ This change is not yet ready to be integrated. See the Progress checklist in the description for automated requirements.

openjdk[bot] avatar Apr 05 '24 19:04 openjdk[bot]

@dcubed-ojdk The hotspot-runtime label was successfully added.

openjdk[bot] avatar Apr 05 '24 19:04 openjdk[bot]

Webrevs

mlbridge[bot] avatar Apr 05 '24 19:04 mlbridge[bot]

Have you thought about expanding this test to virtual threads?

AlanBateman avatar Apr 06 '24 08:04 AlanBateman

@AlanBateman - No I haven't. I'm just "productizing" tests that I use in my stress kit. For this particular test, I adapted an existing lock stack test from @xmas92 and made it into a stress test for recursive locking.

I haven't given any thought to adding virtual threads stress to my stress kit, but that does sound like an "interesting" addition.

dcubed-ojdk avatar Apr 06 '24 12:04 dcubed-ojdk

I haven't given any thought to adding virtual threads stress to my stress kit, but that does sound like an "interesting" addition.

If you refactor SyncThread to wrap rather than extend then it should be straight-forward.

AlanBateman avatar Apr 08 '24 09:04 AlanBateman

I'm finally able to get back to this PR!

@AlanBateman

If you refactor SyncThread to wrap rather than extend then it should be straight-forward.

I won't try to do that in this PR, but can you refer me to an existing test that you consider a good example for using both regular threads and virtual threads? Is there some sort of command line switch option?

dcubed-ojdk avatar Apr 18 '24 21:04 dcubed-ojdk

@dcubed-ojdk This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

bridgekeeper[bot] avatar May 18 '24 02:05 bridgekeeper[bot]