jdk icon indicating copy to clipboard operation
jdk copied to clipboard

8319947: Recursive lightweight locking: s390x implementation

Open offamitkumar opened this issue 1 year ago • 6 comments

s390x port for recursive locking.

testing:

  • [x] build fastdebug-vm
  • [x] build slowdebug-vm
  • [x] build release-vm
  • [x] build optimized-vm
  • [x] ./test/jdk/java/util/concurrent (fastdebug-vm)
    • [x] with C1
    • [x] with C2
    • [x] with interpreter
  • [x] ./test/jdk/java/util/concurrent (release-vm)
    • [x] with C1
    • [x] with C2
    • [x] with interpreter
  • [x] ./test/jdk/java/util/concurrent (slowdebug-vm)
    • [x] with C1
    • [x] with C2
    • [x] with interpreter
  • [x] tier1 with fastdebug-vm
  • [x] tier1 with slowdebug-vm
  • [x] tier1 with release-vm

BenchMarks:

Results from Performance LPARs :

Locking Mode = 1 (without Patch)

Benchmark                                (innerCount)  Mode  Cnt     Score    Error  Units
LockUnlock.testContendedLock                      100  avgt   12     5.144 ±  0.035  ns/op
LockUnlock.testRecursiveLockUnlock                100  avgt   12  3824.742 ± 89.475  ns/op
LockUnlock.testRecursiveSynchronization           100  avgt   12    25.348 ±  0.559  ns/op
LockUnlock.testSerialLockUnlock                   100  avgt   12   466.629 ±  3.036  ns/op
LockUnlock.testSimpleLockUnlock                   100  avgt   12   468.532 ±  1.793  ns/op
Finished running test 'micro:vm.lang.LockUnlock'

Locking Mode = 1 (with patch)

Benchmark                                (innerCount)  Mode  Cnt     Score    Error  Units
LockUnlock.testContendedLock                      100  avgt   12     5.146 ±  0.027  ns/op
LockUnlock.testRecursiveLockUnlock                100  avgt   12  3833.175 ± 75.863  ns/op
LockUnlock.testRecursiveSynchronization           100  avgt   12    25.206 ±  0.519  ns/op
LockUnlock.testSerialLockUnlock                   100  avgt   12   473.973 ±  2.103  ns/op
LockUnlock.testSimpleLockUnlock                   100  avgt   12   470.749 ±  2.229  ns/op
Finished running test 'micro:vm.lang.LockUnlock'

Locking Mode = 2  (without Patch)

Benchmark                                (innerCount)  Mode  Cnt      Score    Error  Units
LockUnlock.testContendedLock                      100  avgt   12      4.688 ±  0.051  ns/op
LockUnlock.testRecursiveLockUnlock                100  avgt   12  12800.544 ± 92.265  ns/op
LockUnlock.testRecursiveSynchronization           100  avgt   12     26.486 ±  2.229  ns/op
LockUnlock.testSerialLockUnlock                   100  avgt   12    424.499 ±  0.416  ns/op
LockUnlock.testSimpleLockUnlock                   100  avgt   12    424.241 ±  0.840  ns/op
Finished running test 'micro:vm.lang.LockUnlock'

Locking Mode = 2 (with patch)

Benchmark                                (innerCount)  Mode  Cnt     Score    Error  Units
LockUnlock.testContendedLock                      100  avgt   12     5.447 ±  0.069  ns/op
LockUnlock.testRecursiveLockUnlock                100  avgt   12  4054.898 ± 84.878  ns/op
LockUnlock.testRecursiveSynchronization           100  avgt   12    26.374 ±  0.051  ns/op
LockUnlock.testSerialLockUnlock                   100  avgt   12   426.174 ±  2.181  ns/op
LockUnlock.testSimpleLockUnlock                   100  avgt   12   424.974 ±  0.680  ns/op
Finished running test 'micro:vm.lang.LockUnlock'

PPC port for the same: https://github.com/openjdk/jdk/pull/16611


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

Issue

  • JDK-8319947: Recursive lightweight locking: s390x implementation (Enhancement - P4)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 18878

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

Using diff file

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

Webrev

Link to Webrev Comment

offamitkumar avatar Apr 21 '24 16:04 offamitkumar

:wave: Welcome back amitkumar! 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 21 '24 16:04 bridgekeeper[bot]

@offamitkumar This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8319947: Recursive lightweight locking: s390x implementation

Reviewed-by: aboldtch, lucy

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 98 new commits pushed to the master branch:

  • 75a2afacc8f5fdec53350b1cb66076cdfeae12f0: 8248981: Specify list of standard message digest and mgf algorithms for RSASSA-PSS signature
  • baafa662a2f0706e4275a4fe0459ee6759369858: 8334287: Man page update for jstatd deprecation
  • c30e040342c69a213bdff321fdcb0d27ff740489: 8331911: Reconsider locking for recently disarmed nmethods
  • 974dca80df71c5cbe492d1e8ca5cee76bcc79358: 8334223: Make Arena MEMFLAGs immutable
  • e527e1c32fcc7b2560cec540bcde930075ac284a: 8334580: Deprecate no-arg constructor BasicSliderUI() for removal
  • 3a26bbcebc2f7d11b172f2b16192a3adefeb8111: 8185429: [macos] After a modal dialog is closed, no window becomes active
  • 4b153e5e051c01ad8d0c3ff335352918c2970fe6: 8306580: Propagate CDS dumping errors instead of directly exiting the VM
  • 71a692ab435fdeea4ce8f8db7a55dd735c7c5016: 8321033: Avoid casting Array to GrowableArray
  • 55c796946158aab1d019a57b77a33441d7b13065: 8334765: JFR: Log chunk waste
  • b2930c5aeedf911ec893734181c1af0573e222f4: 8334040: jdk/classfile/CorpusTest.java timed out
  • ... and 88 more: https://git.openjdk.org/jdk/compare/8464ce6db5cbd5d50ac2a2bcba905b7255f510f5...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

openjdk[bot] avatar Apr 21 '24 16:04 openjdk[bot]

@offamitkumar The following label will be automatically applied to this pull request:

  • hotspot

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

openjdk[bot] avatar Apr 21 '24 16:04 openjdk[bot]

JDK-8330849 adds TestRecursiveMonitorChurn.java. So rebasing the whole code to fetch that test.

offamitkumar avatar Apr 28 '24 05:04 offamitkumar

@TheRealMDoerr Would you please review this one :-)

offamitkumar avatar May 19 '24 10:05 offamitkumar

Thanks Axel for review.

let's wait for @TheRealMDoerr & @RealLucy comments ;-)

offamitkumar avatar May 25 '24 08:05 offamitkumar

@TheRealMDoerr would you take final look, before I integrate it :-)

offamitkumar avatar Jun 25 '24 13:06 offamitkumar

Thanks Lutz, Axel for the reviews. I did one more round of testing and things seems fine.

/integrate

offamitkumar avatar Jun 28 '24 06:06 offamitkumar

Going to push as commit d457609f700bbb1fed233f1a04501c995852e5ac. Since your change was applied there have been 154 commits pushed to the master branch:

  • c47a0e005e54551e42ee1ae33d7169417a5f86d4: 8334147: Shenandoah: Avoid taking lock for disabled free set logging
  • 308a81238362c39f5b18e2ae8444c96420ef297a: 8334645: Un-problemlist vmTestbase/nsk/sysdict/vm/stress/chain/chain007/chain007.java
  • b4df380f1a4587247a843fe28ae041265f7cfc29: 8334763: --enable-asan: assert(_thread->is_in_live_stack((address)this)) failed: not on stack?
  • cd46c87dc916b2b74067accf80c62df1792f74cf: 8334843: RISC-V: Fix wraparound checking for r_array_index in lookup_secondary_supers_table_slow_path
  • 4e8cbf884ab1eee9c3110712ab62edc706e948ba: 8335134: Test com/sun/jdi/BreakpointOnClassPrepare.java timeout
  • 3b1ca986427d3a69c9e167b9b4c07d1b83bc264d: 8334895: OpenJDK fails to configure on linux aarch64 when CDS is disabled after JDK-8331942
  • c35e58a5adf06e25a3b482e2be384af95a84f11a: 8309634: Resolve CONSTANT_MethodRef at CDS dump time
  • 243bae7dc0c3e71c02ffed9e1ee7d436af11d3b9: 8304693: Remove -XX:-UseVtableBasedCHA
  • 9d986a013d01a5bcc0942bcc490258038291c22c: 8335220: C2: Missing check for Opaque4 node in EscapeAnalysis
  • 0e6b0cbaaa0d5272f60ee4fe09cf5e247e68c2a8: 8334886: jdk/jfr/api/recording/time/TestTimeMultiple.java failed with RuntimeException: getStopTime() > afterStop
  • ... and 144 more: https://git.openjdk.org/jdk/compare/8464ce6db5cbd5d50ac2a2bcba905b7255f510f5...master

Your commit was automatically rebased without conflicts.

openjdk[bot] avatar Jun 28 '24 06:06 openjdk[bot]

@offamitkumar Pushed as commit d457609f700bbb1fed233f1a04501c995852e5ac.

:bulb: You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

openjdk[bot] avatar Jun 28 '24 06:06 openjdk[bot]