8278411: Implement UseHeavyMonitors consistently, s390 port #13978

offamitkumar · 2023-05-15T08:25:19Z

This PR make s390x to adapt the changes done in JDK-8276901 OR implements UseHeavyMonitors.

JDK-8291555 still needs Porting effort. As for LM_LIGHTWEIGHT locking mode, code is Unimplemented.

Progress

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

Issue

JDK-8278411: Implement UseHeavyMonitors consistently, s390 port

Reviewers

Martin Doerr (@TheRealMDoerr - Reviewer)
Thomas Stuefe (@tstuefe - Reviewer)
Lutz Schmidt (@RealLucy - Reviewer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 13978

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

Using diff file

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

Webrev

Link to Webrev Comment

bridgekeeper · 2023-05-15T08:26:06Z

👋 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.

openjdk · 2023-05-15T08:28:57Z

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

core-libs
hotspot

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

mlbridge · 2023-05-15T08:32:49Z

Webrevs

01: Full - Incremental (c06afdb9)
00: Full (f5af1367)

offamitkumar · 2023-05-15T08:38:13Z

/label remove core-libs, hotspot

/label add hotspot-compiler, hotspot-runtime

openjdk · 2023-05-15T08:39:23Z

@offamitkumar
The core-libs label was successfully removed.

The hotspot label was successfully removed.

openjdk · 2023-05-15T08:40:54Z

@offamitkumar
The hotspot-compiler label was successfully added.

The hotspot-runtime label was successfully added.

src/hotspot/cpu/s390/c1_MacroAssembler_s390.cpp

src/hotspot/cpu/s390/macroAssembler_s390.cpp

TheRealMDoerr

Please check my findings.

TheRealMDoerr · 2023-05-15T12:51:13Z

src/hotspot/cpu/s390/macroAssembler_s390.cpp

+    z_bre(done);
+  } else {
+    // Set NE to indicate 'failure' -> take slow-path
+    z_ltgr(oop, oop);


I think z_bru(done); is missing, here.

Please check updated version.

TheRealMDoerr · 2023-05-15T12:52:01Z

src/hotspot/cpu/s390/macroAssembler_s390.cpp

+    z_bru(done); // Csg sets CR as desired.
+  } else {
+    // Set NE to indicate 'failure' -> take slow-path
+    z_ltgr(oop, oop);


Same, here.

tstuefe

LGTM

openjdk · 2023-05-16T07:08:41Z

@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:

8278411: Implement UseHeavyMonitors consistently, s390 port

Reviewed-by: mdoerr, stuefe, 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 23 new commits pushed to the master branch:

316bc79: 8308188: ProblemList java/util/concurrent/locks/Lock/OOMEInAQS.java with ZGC on all platforms
8686a36: 8307315: Missing ResourceMark in CDS and JVMTI code
23cbb2d: 8306860: Avoid unnecessary allocation in List.map() when list is empty
be54b54: 8307369: Add execution of all svc tests in CI
19c8c30: 8305634: Renaming predicates, simple cleanups, and adding summary about current predicates
72294c5: 8308024: HttpClient (HTTP/1.1) sends an extraneous empty chunk if the BodyPublisher supplies an empty buffer
c9b6bb5: 8308041: [JVMCI] WB_IsGCSupportedByJVMCICompiler must enter correct JVMCI env
0790f70: 8306806: JMX agent with JDP enabled won't start when PerfData is disabled
c2ef302: 8307968: serviceability/jvmti/vthread/StopThreadTest/StopThreadTest.java timed out
d22bcc8: 8307403: java/util/zip/DeInflate.java timed out
... and 13 more: https://git.openjdk.org/jdk/compare/8d49ba9e8d3095f850b3007b56488a0c0cf8ddff...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.

As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@TheRealMDoerr, @tstuefe, @RealLucy) but any other Committer may sponsor as well.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

offamitkumar · 2023-05-16T07:25:06Z

Thanks @tstuefe for Review.

@RealLucy would you like to review it as well.

RealLucy

LGTM.

offamitkumar · 2023-05-16T13:08:31Z

Thank you @RealLucy for reviewing it.

@TheRealMDoerr would you like to take another look before we proceed with integration 🙂

TheRealMDoerr

Thanks for the update!

offamitkumar · 2023-05-16T15:15:01Z

Thanks for reviews,

/integrate

openjdk · 2023-05-16T15:16:32Z

@offamitkumar
Your change (at version c06afdb) is now ready to be sponsored by a Committer.

TheRealMDoerr · 2023-05-16T15:24:12Z

/sponsor

openjdk · 2023-05-16T15:25:28Z

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

316bc79: 8308188: ProblemList java/util/concurrent/locks/Lock/OOMEInAQS.java with ZGC on all platforms
8686a36: 8307315: Missing ResourceMark in CDS and JVMTI code
23cbb2d: 8306860: Avoid unnecessary allocation in List.map() when list is empty
be54b54: 8307369: Add execution of all svc tests in CI
19c8c30: 8305634: Renaming predicates, simple cleanups, and adding summary about current predicates
72294c5: 8308024: HttpClient (HTTP/1.1) sends an extraneous empty chunk if the BodyPublisher supplies an empty buffer
c9b6bb5: 8308041: [JVMCI] WB_IsGCSupportedByJVMCICompiler must enter correct JVMCI env
0790f70: 8306806: JMX agent with JDP enabled won't start when PerfData is disabled
c2ef302: 8307968: serviceability/jvmti/vthread/StopThreadTest/StopThreadTest.java timed out
d22bcc8: 8307403: java/util/zip/DeInflate.java timed out
... and 13 more: https://git.openjdk.org/jdk/compare/8d49ba9e8d3095f850b3007b56488a0c0cf8ddff...master

Your commit was automatically rebased without conflicts.

openjdk · 2023-05-16T15:25:44Z

@TheRealMDoerr @offamitkumar Pushed as commit 41ee125.

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

s390x Port

f5af136

openjdk bot changed the title ~~8278411~~ 8278411: Implement UseHeavyMonitors consistently, s390 port May 15, 2023

openjdk bot added the rfr Pull request is ready for review label May 15, 2023

openjdk bot added hotspot hotspot-dev@openjdk.org core-libs core-libs-dev@openjdk.org labels May 15, 2023

openjdk bot removed core-libs core-libs-dev@openjdk.org hotspot hotspot-dev@openjdk.org labels May 15, 2023

openjdk bot added hotspot-compiler hotspot-compiler-dev@openjdk.org hotspot-runtime hotspot-runtime-dev@openjdk.org labels May 15, 2023

tstuefe reviewed May 15, 2023

View reviewed changes

src/hotspot/cpu/s390/c1_MacroAssembler_s390.cpp Show resolved Hide resolved

src/hotspot/cpu/s390/macroAssembler_s390.cpp Show resolved Hide resolved

src/hotspot/cpu/s390/macroAssembler_s390.cpp Show resolved Hide resolved

TheRealMDoerr reviewed May 15, 2023

View reviewed changes

suggestion from @TheRealMDoerr and @tstuefe

c06afdb

tstuefe approved these changes May 16, 2023

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label May 16, 2023

RealLucy approved these changes May 16, 2023

View reviewed changes

TheRealMDoerr approved these changes May 16, 2023

View reviewed changes

openjdk bot added the sponsor Pull request is ready to be sponsored label May 16, 2023

openjdk bot added the integrated Pull request has been integrated label May 16, 2023

openjdk bot closed this May 16, 2023

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review sponsor Pull request is ready to be sponsored labels May 16, 2023

offamitkumar deleted the HeavyLightMonitor_8278411 branch May 24, 2023 09:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8278411: Implement UseHeavyMonitors consistently, s390 port #13978

8278411: Implement UseHeavyMonitors consistently, s390 port #13978

offamitkumar commented May 15, 2023 •

edited by openjdk bot

Loading

bridgekeeper bot commented May 15, 2023

openjdk bot commented May 15, 2023

mlbridge bot commented May 15, 2023 •

edited

Loading

offamitkumar commented May 15, 2023

openjdk bot commented May 15, 2023

openjdk bot commented May 15, 2023

TheRealMDoerr left a comment

TheRealMDoerr May 15, 2023

offamitkumar May 16, 2023

TheRealMDoerr May 15, 2023

tstuefe left a comment

openjdk bot commented May 16, 2023 •

edited

Loading

offamitkumar commented May 16, 2023

RealLucy left a comment

offamitkumar commented May 16, 2023

TheRealMDoerr left a comment

offamitkumar commented May 16, 2023

openjdk bot commented May 16, 2023

TheRealMDoerr commented May 16, 2023

openjdk bot commented May 16, 2023

openjdk bot commented May 16, 2023

8278411: Implement UseHeavyMonitors consistently, s390 port #13978

8278411: Implement UseHeavyMonitors consistently, s390 port #13978

Conversation

offamitkumar commented May 15, 2023 • edited by openjdk bot Loading

Progress

Issue

Reviewers

Reviewing

Webrev

bridgekeeper bot commented May 15, 2023

openjdk bot commented May 15, 2023

mlbridge bot commented May 15, 2023 • edited Loading

Webrevs

offamitkumar commented May 15, 2023

openjdk bot commented May 15, 2023

openjdk bot commented May 15, 2023

TheRealMDoerr left a comment

Choose a reason for hiding this comment

TheRealMDoerr May 15, 2023

Choose a reason for hiding this comment

offamitkumar May 16, 2023

Choose a reason for hiding this comment

TheRealMDoerr May 15, 2023

Choose a reason for hiding this comment

tstuefe left a comment

Choose a reason for hiding this comment

openjdk bot commented May 16, 2023 • edited Loading

offamitkumar commented May 16, 2023

RealLucy left a comment

Choose a reason for hiding this comment

offamitkumar commented May 16, 2023

TheRealMDoerr left a comment

Choose a reason for hiding this comment

offamitkumar commented May 16, 2023

openjdk bot commented May 16, 2023

TheRealMDoerr commented May 16, 2023

openjdk bot commented May 16, 2023

openjdk bot commented May 16, 2023

offamitkumar commented May 15, 2023 •

edited by openjdk bot

Loading

mlbridge bot commented May 15, 2023 •

edited

Loading

openjdk bot commented May 16, 2023 •

edited

Loading