8371327: G1: Replace FromCardCache with thread-local per-card cache by walulyai · Pull Request #32479 · openjdk/jdk

walulyai · 2026-08-21T10:14:39Z

Hi,

Please review this change that replaces the global direct-indexed G1FromCardCache with a per-worker local cache.

After JEP 522, additions to G1CardSets are performed during linear heap scans: Concurrent refinement sweeps and Concurrent remset rebuild. The linear traversal does not need to cache state for previously processed cards. We can maintain only the cache state for the current card, then discard the state when traversal advances.

The new cache maintains the current source card and an array of cardsets already visited for that card. This reduces the memory overhead of the FromCardCache from O(number of regions × number of workers) to O(maximum references per card × number of active workers).

The old cache provided constant-time lookups, while the new cache linearly searches the array of cardsets. These arrays are normally short (the benchmarks show approximately 1.3 comparisons per lookup) and in the worst case bounded by the maximum references per card.

Testing:

Tier 1-3
No performance regression was observed in the tested workloads.

I confirm that I make this contribution in accordance with the OpenJDK Interim AI Policy.

Progress

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

Issue

JDK-8371327: G1: Replace FromCardCache with thread-local per-card cache (Enhancement - P4)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 32479

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

Using diff file

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

Using Webrev

Link to Webrev Comment

bridgekeeper · 2026-08-21T10:16:28Z

👋 Welcome back iwalulya! 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 · 2026-08-21T10:17:07Z

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

openjdk · 2026-08-21T10:17:57Z

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

hotspot-gc

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 · 2026-08-21T10:18:00Z

The total number of required reviews for this PR has been set to 2 based on the presence of this label: hotspot-gc. This can be overridden with the /reviewers command.

mlbridge · 2026-08-21T11:34:47Z

Webrevs

01: Full - Incremental (811d3561)
00: Full (ab4549df)

tschatzl

Can you provide some more statistics on refinement time/number of iterations in that new FCC? Like benchmark/refinement time before/after, avg number of iterations to search

tschatzl · 2026-08-24T14:10:17Z

+        if (_cm->do_yield_check()) {
+          _rebuild_closure.reset_from_card_cache();
+          if (!should_rebuild_or_scrub(hr)) {


Just a suggestion, maybe encapsulate the yield check with the reset, i.e. something like:

bool my_yield_check() { bool yielded = _cm->do_yield_check(); if (yielded) { _rebuild_closure.reset_from_card_cache(); } return yielded; }

Maybe not though.

tschatzl · 2026-08-24T14:11:59Z

    verify_card_pair_refers_to_same_card(source_card, dest_card);

-    G1RemSet::RefineResult res = _rem_set->refine_card_concurrently(source_card, _worker_id);
+    G1RemSet::RefineResult res = _rem_set->refine_card_concurrently(source_card, _conc_refine_cl);


Did you consider putting the _conc_refine_cl construction into refine_card_concurrently()? Its lifecycle is exactly the same as that method after all.

Yes, started out with G1ConcurrentRefineOopClosure conc_refine_cl local to the refine_card_concurrently, but maybe hoisting it was "premature" optimization.

tschatzl · 2026-08-24T14:14:25Z

-  // This card index indicates "no card for that entry" yet. This allows us to use the OS
-  // lazy backing of memory with zero-filled pages to avoid initial actual memory use.
-  // This means that the heap must not contain card zero.
-  static const uintptr_t InvalidCard = 0;


I would prefer if that constant were explicitly retained for clarity. Its comment explains why its use is safe...

tschatzl · 2026-08-24T14:25:23Z

+
+  assert(_num_cardsets < MaxNumCardsets, "source card has too many destination cardsets");
+
+  if (_num_cardsets < MaxNumCardsets) {


I think this check is redundant and/or harmful. First, there is that assert above, and loosing a card just makes the program crash.

I would make sure that MaxCardSizeInBytes is consistent with the flag, and only add the assert.

Applied from openjdk/jdk PR openjdk#32479 by Ivan Walulya. After JEP 522, additions to G1CardSets happen during linear heap scans, so there is no need to cache state for previously processed cards. Replace the global O(regions x workers) direct-indexed FromCardCache with a per-closure cache holding the current source card and the cardset ids already visited for it, keyed on the cset candidate group id.

walulyai added 12 commits August 12, 2026 10:34

Init

2b36b57

cleanup

acbfe24

init

dbbfd61

Change to using Arrays

7077285

Merge remote-tracking branch 'upstream/master' into FromCardCache_v2

d1fdd3d

cleanup

2fa00bc

change to thread stack caches

d4173a9

hoist G1ConcurrentRefineOopClosure

80c772e

cleanup group ids

4676588

Add trivial test

6e35ddc

Merge remote-tracking branch 'upstream/master' into FromCardCache_v2

11d45a2

cleanup

ab4549d

openjdk Bot added the hotspot-gc hotspot-gc-dev@openjdk.org label Aug 21, 2026

walulyai marked this pull request as ready for review August 21, 2026 11:29

openjdk Bot added the rfr Pull request is ready for review label Aug 21, 2026

tschatzl suggested changes Aug 24, 2026

View reviewed changes

Thomas review

811d356


		assert(_num_cardsets < MaxNumCardsets, "source card has too many destination cardsets");

		if (_num_cardsets < MaxNumCardsets) {

Uh oh!

Conversation

walulyai commented Aug 21, 2026 • edited by openjdk Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewing

Uh oh!

bridgekeeper Bot commented Aug 21, 2026

Uh oh!

openjdk Bot commented Aug 21, 2026

Uh oh!

openjdk Bot commented Aug 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openjdk Bot commented Aug 21, 2026

Uh oh!

mlbridge Bot commented Aug 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

tschatzl left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tschatzl Aug 24, 2026

Choose a reason for hiding this comment

Uh oh!

tschatzl Aug 24, 2026

Choose a reason for hiding this comment

Uh oh!

walulyai Aug 24, 2026

Choose a reason for hiding this comment

Uh oh!

tschatzl Aug 24, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tschatzl Aug 24, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

walulyai commented Aug 21, 2026 •

edited by openjdk Bot

Loading

openjdk Bot commented Aug 21, 2026 •

edited

Loading

mlbridge Bot commented Aug 21, 2026 •

edited

Loading

tschatzl left a comment •

edited

Loading