8343468: GenShen: Enable relocation of remembered set card tables

[JohnTortugo]

In the current Generational Shenandoah implementation, the pointers to the read and write card tables are established at JVM launch time and fixed during the whole of the application execution. Because they are considered constants, they are embedded as such in JIT-compiled code.

The cleaning of dirty cards in the read card table is performed during the init-mark pause, and our experiments show that it represents a sizable portion of that phase's duration. This pull request makes the addresses of the read and write card tables dynamic, with the end goal of reducing the duration of the init-mark pause by moving the cleaning of the dirty cards in the read card table to the reset concurrent phase.

The idea is quite simple. Instead of using distinct read and write card tables for the entire duration of the JVM execution, we alternate which card table serves as the read/write table during each GC cycle. In the reset phase we concurrently clean the cards in the the current read table so that when the cycle reaches the next init-mark phase we have a version of the card table totally clear. In the next init-mark pause we swap the pointers to the base of the read and write tables. When the init-mark finishes the mutator threads will operate on the table just cleaned in the reset phase; the GC will operate on the table that just turned the new read table.

Most of the changes in the patch account for the fact that the write card table is no longer at a fixed address.

The primary benefit of this change is that it eliminates the need to copy and zero the remembered set during the init-mark Safepoint. A secondary benefit is that it allows us to replace the init-mark Safepoint with an init-mark handshake—something we plan to work on after this PR is merged.

Our internal performance testing showed a significant reduction in the duration of init-mark pauses and no statistically significant regression due to the dynamic loading of the card table address in JIT-compiled code.

Functional testing was performed on Linux, macOS, Windows running on x64, AArch64, and their respective 32-bit versions. I’d appreciate it if someone with access to RISC-V (@luhenry ?) and PowerPC (@TheRealMDoerr ?) platforms could review and test the changes for those platforms, as I have limited access to running tests on them.

---

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-8343468: GenShen: Enable relocation of remembered set card tables (Enhancement - P4)

Reviewers

• Aleksey Shipilev (@shipilev - Reviewer)
• Kelvin Nilsen (@kdnilsen - Committer) Review applies to 4b00cfec
• William Kemper (@earthling-amzn - Reviewer) Review applies to 4b00cfec

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 23170

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/23170.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:

8343468: GenShen: Enable relocation of remembered set card tables

Reviewed-by: shade, 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 26 new commits pushed to the master branch:

• a23fb0a: 8348110: Update LCMS to 2.17
• 0892913: 8351086: (fc) Make java/nio/channels/FileChannel/BlockDeviceSize.java test manual
• 93f2260: 8351294: (fs) Minor verbiage correction for Files.createTemp{Directory,File}
• 8f8a879: 8350939: Revisit Windows PDH buffer size calculation for OperatingSystemMXBean
• cfab88b: 8351256: Improve printing of runtime call stub names in disassember output
• 7a5acb9: 8343840: Rewrite the ObjectMonitor lists
• 40f150d: 8330936: [ubsan] exclude function BilinearInterp and ShapeSINextSpan in libawt java2d from ubsan checks
• 649ef77: 8323158: HotSpot Style Guide should specify more include ordering
• 5c552a9: 8349358: [JMH] Cannot access class jdk.internal.vm.ContinuationScope
• e82031e: 8350756: C2 SuperWord Multiversioning: remove useless slow loop when the fast loop disappears
• ... and 16 more: https://git.openjdk.org/jdk/compare/20ea218ce52f79704445acfe2d4a3dc9d04e86d2...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 (@shipilev, @kdnilsen, @earthling-amzn) 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).

[openjdk]

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

• 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

• 08: Full - Incremental (0a540c79)
• 07: Full - Incremental (6ebc6e5b)
• 06: Full (046ea8a0)
• 05: Full - Incremental (cbf5aab0)
• 04: Full - Incremental (717b8b44)
• 03: Full (6210f026)
• 02: Full (4b00cfec)
• 01: Full - Incremental (124c5f70)
• 00: Full (87a9993a)

[shipilev]

I do wonder if you can avoid a lot if not all arch-specific changes, if Shenandoah barrier set: a) does not use MacroAssembler::load_byte_map_base directly; b) returns deliberately bad pointer from byte_map_base(), so that C2 matcher would never match, and accidental use of byte_map_base() would cleanly crash.

[ysramakrishna]

Thanks for implementing this very important feature which already improves performance (and which will further unlock better performance when init-mark becomes a per-thread handshake in the future).

Our internal performance testing showed a significant reduction in the duration of init-mark pauses and no statistically significant regression due to the dynamic loading of the card table address in JIT-compiled code.

It would be great if you can include some performance numbers either in this PR or, preferably, in the JBS ticket.

Thanks!

[kdnilsen]

Thanks for figuring out how to make all of this work. It represents a very significant improvement to the GenShen implementation.

[JohnTortugo]

I do wonder if you can avoid a lot if not all arch-specific changes, if Shenandoah barrier set: a) does not use MacroAssembler::load_byte_map_base directly; b) returns deliberately bad pointer from byte_map_base(), so that C2 matcher would never match, and accidental use of byte_map_base() would cleanly crash.

@shipilev - are you suggesting to patch all shenandoahBarrierSetAssembler_* backend specific files to not use load_byte_map? It looks like we would end up in a similar situation if I go with the approach that you're suggesting - if I'm understanding it correctly.

[shipilev]

I do wonder if you can avoid a lot if not all arch-specific changes, if Shenandoah barrier set: a) does not use MacroAssembler::load_byte_map_base directly; b) returns deliberately bad pointer from byte_map_base(), so that C2 matcher would never match, and accidental use of byte_map_base() would cleanly crash.

@shipilev - are you suggesting to patch all shenandoahBarrierSetAssembler_* backend specific files to not use load_byte_map? It looks like we would end up in a similar situation if I go with the approach that you're suggesting - if I'm understanding it correctly.

Yes, I was thinking that if Shenandoah cannot use super-class load_byte_map and needs its own implementation, then it makes sense that Shenandoah barrier set should be using a special implementation directly instead of hacking the super-class implementation?

But mostly I want to avoid unnecessary changes in files that are not related to Shenandoah :)

[openjdk]

@JohnTortugo this pull request can not be integrated into master due to one or more merge conflicts. To resolve these merge conflicts and update this pull request you can run the following commands in the local repository for your personal fork:

git checkout relocation_of_card_tables
git fetch https://git.openjdk.org/jdk.git master
git merge FETCH_HEAD
# resolve conflicts and follow the instructions given by git merge
git commit -m "Merge master"
git push

[JohnTortugo]

I do wonder if you can avoid a lot if not all arch-specific changes, if Shenandoah barrier set: a) does not use MacroAssembler::load_byte_map_base directly; b) returns deliberately bad pointer from byte_map_base(), so that C2 matcher would never match, and accidental use of byte_map_base() would cleanly crash.

@shipilev - are you suggesting to patch all shenandoahBarrierSetAssembler_* backend specific files to not use load_byte_map? It looks like we would end up in a similar situation if I go with the approach that you're suggesting - if I'm understanding it correctly.

Yes, I was thinking that if Shenandoah cannot use super-class load_byte_map and needs its own implementation, then it makes sense that Shenandoah barrier set should be using a special implementation directly instead of hacking the super-class implementation?

But mostly I want to avoid unnecessary changes in files that are not related to Shenandoah :)

@shipilev - Please, take a look at the latest changes when you can.

It would be great if you can include some performance numbers either in this PR or, preferably, in the JBS ticket.

@ysramakrishna - I'm working on that!

[TheRealMDoerr]

The ppc fix looks good. Thanks! I guess we should rerun tests shortly before integration. There are currently many Shenandoah issues (mostly related to GenShen).

[JohnTortugo]

There are currently many Shenandoah issues (mostly related to GenShen).

@TheRealMDoerr - are the issues coming from a build with this patch or something prior?

[TheRealMDoerr]

There are currently many Shenandoah issues (mostly related to GenShen).

@TheRealMDoerr - are the issues coming from a build with this patch or something prior?

Before this patch. We have linked the issues we found with JDK-8337511.

[RealFYang]

FYI: gtest:all and hotspot_gc_shenandoah are clean on linux-riscv64 platform with this change.

[TheRealMDoerr]

I have run this PR through our nightly tests on all our platforms and there were no new issues.

[JohnTortugo]

@ysramakrishna - I attached to the RFE a graph showing the reduction in init-pause duration when executing SPECJBB2015 on x86_64.

[JohnTortugo]

Anyone has any other concern / comment ?

[kdnilsen]

Thanks for pulling this together. Looks great.

[shipilev]

I'll take a look at this tomorrow, thanks.

[shipilev]

Much cleaner, thanks! I'll take another look later, but meanwhile, some comments:

[shipilev]

Looks fine now, thanks! I have not looked deeply at card table lifecycle, so I rely on @kdnilsen and @earthling-amzn reviews here.

[JohnTortugo]

/integrate

[openjdk]

@JohnTortugo
Your change (at version 0a540c7) is now ready to be sponsored by a Committer.

[sendaoYan]

/sponsor

[openjdk]

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

• a90f323: 8349705: java.net.URI.scanIPv4Address throws unnecessary URISyntaxException
• 7ec2e14: 8349932: PSPrinterJob sometimes generates unnecessary PostScript commands
• 76e0f30: 8350460: org.openjdk.bench.vm.floatingpoint.DremFrem JMH fails with -ea
• 4e67ac4: 8350909: [JMH] test ThreadOnSpinWaitShared failed for 2 threads config
• 8ed6c1d: 8350607: Consolidate MethodHandles::zero into MethodHandles::constant
• f6a8db2: 8348261: assert(n->is_Mem()) failed: memory node required
• 5cd4fe6: 8348309: MultiNST tests need more debugging and timing
• 7c22b81: 8350811: [JMH] test foreign.StrLenTest failed with StringIndexOutOfBoundsException for size=451
• 54fe643: 8347433: Deprecate XML interchange in java.management/javax/management/modelmbean/DescriptorSupport for removal
• 155697f: 8349623: [ASAN] Gtest os_linux.glibc_mallinfo_wrapper_vm fails
• ... and 29 more: https://git.openjdk.org/jdk/compare/20ea218ce52f79704445acfe2d4a3dc9d04e86d2...master

Your commit was automatically rebased without conflicts.

[openjdk]

@sendaoYan @JohnTortugo Pushed as commit 4e1367e.

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

[AlanBateman]

This seems to be break Oracle --disable-jvm-feature-shenandoahgc builds on aarch64.

[2025-03-08T14:16:00,338Z] src/hotspot/cpu/aarch64/aarch64.ad:4544:58: error: no member named 'ShenandoahBarrierSet' in 'BarrierSet'
[2025-03-08T14:16:00,338Z]             !BarrierSet::barrier_set()->is_a(BarrierSet::ShenandoahBarrierSet) &&