8364936: Shenandoah: Switch nmethod entry barriers to conc_instruction_and_data_patch

[JohnTortugo]

Please, review this patch to make nmethod entry barriers use conc-instruction-and-data-patch fence mechanics when ShenandoahGC is being used on AArch64. The patch also removes (including from JVMCI interface) the old constant that was being used only by Shenandoah on AArch64.

The patch has been tested with functional and performance benchmarks on AArch64. Improvements in DaCapo and Renaissance benchmarks can be as high as 30%. Maximum critical Jops in SPEC improved by ~10%.

---

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-8364936: Shenandoah: Switch nmethod entry barriers to conc_instruction_and_data_patch (Enhancement - P4)

Reviewers

• Kelvin Nilsen (@kdnilsen - Committer) Review applies to 62da55fb
• William Kemper (@earthling-amzn - Reviewer) Review applies to 62da55fb
• Fei Yang (@RealFYang - Reviewer)
• Dingli Zhang (@DingliZhang - Author)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 26999

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

Using diff file

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

Using Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back cslucas! 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]

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

8364936: Shenandoah: Switch nmethod entry barriers to conc_instruction_and_data_patch

Reviewed-by: fyang, dzhang, kdnilsen, wkemper

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 229 new commits pushed to the master branch:

• e268563: 8366476: Test gc/z/TestSmallHeap.java fails OOM with many NUMA nodes
• 5052a7e: 8246037: Shenandoah: update man pages to mention -XX:+UseShenandoahGC
• b50c11f: 8366195: Remove unnecessary quotes around -Ta ml64 assembler argument
• ... and 226 more: https://git.openjdk.org/jdk/compare/57210af9bceb582be112564465ab66cebd43a4c0...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]

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

• graal
• hotspot
• shenandoah

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]

Webrevs

• 02: Full - Incremental (4da31385)
• 01: Full - Incremental (3276b586)
• 00: Full (62da55fb)

[shipilev]

I see virtual NMethodPatchingType nmethod_patching_type() { return NMethodPatchingType::conc_data_patch; } in Shenandoah PPC64 and RISC-V barrier sets as well. Those should likely go away as well?

[DingliZhang]

Thanks for this patch! I've made similar changes on RISC-V.
Could you please add those as well? DingliZhang@495b07f
I've tested the hotspot_gc_shenandoah on the SG2042 with fastdebug and found no failures.
SPECjbb on the SG2042 is ongoing. I'll update the results later.

PS: max-jOPS in SPEC improved by ~5% and critical-jOPS in SPEC improved by ~3% on SG2042.

[kdnilsen]

Thank you for doing this. Looks good to me.

[earthling-amzn]

LGTM

[RealFYang]

I just checked the RISC-V part. LGTM. Thanks.

[DingliZhang]

LGTM, thanks!

[JohnTortugo]

@TheRealMDoerr - @shipilev mentioned in private that you have access to PPC machines; could you please help testing this patch on PPC?

[JohnTortugo]

@TheRealMDoerr - gentle nudge.

[JohnTortugo]

@dbriemann - do you have access to a PPC machine that you can use to test this patch? TIA.

[TheRealMDoerr]

This change is a NOP for PPC64 since we're generating the same code for both patching types. GHA cross build has passed, so there's nothing to worry regarding PPC64.

[JohnTortugo]

/integrate

[openjdk]

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

• 56e3735: 8367130: JDK builds broken by 8366837: Clean up gensrc by spp.Spp
• 3e68d7d: 8366881: Parallel: Obsolete HeapMaximumCompactionInterval
• 85441ce: 8367101: Remove unused includes in cardTable.cpp
• ... and 306 more: https://git.openjdk.org/jdk/compare/57210af9bceb582be112564465ab66cebd43a4c0...master

Your commit was automatically rebased without conflicts.

[openjdk]

@JohnTortugo Pushed as commit 81a1e8e.

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