8372543: Shenandoah: undercalculated the available size when soft max takes effect by rgithubli · Pull Request #28622 · openjdk/jdk

rgithubli · 2025-12-03T02:02:18Z

Detailed math and repro see https://bugs.openjdk.org/browse/JDK-8372543.

Currently in shenandoah, when deciding whether to have gc, how we calculate available size is:

available = (Xmx * (100 - ShenandoahEvacReserve) / 100) - used
soft_tail = Xmx - soft_max
if (available - soft_tail < ShenandoahMinFreeThreshold * soft_max) // trigger gc

The if condition available - soft_tail will be reduced to: -(ShenandoahEvacReserve/100) * Xmx - used + soft_max, which means when soft max is the same, the larger Xmx is, the less free size the app would have and the more gc it would have, which does not make sense, especially for the case where the app is mostly idle. This caused one of our internal customers experienced frequent gc with minimal workload, when soft max heap size was set way lower than Xmx.

Suggested fix: when deciding when to trigger gc, use logic similar to below:

mutator_soft_capacity = soft_max * (100 - ShenandoahEvacReserve) / 100;
available = mutator_soft_capacity - used;
if (available < mutator_soft_capacity) // trigger gc

Tests:

Ran the repro app StableLiveSet.java in https://bugs.openjdk.org/browse/JDK-8372543. Without fix, tip had ~2910 times gc in 20 sec with -XX:+UseShenandoahGC -XX:ShenandoahGCHeuristics=adaptive -XX:SoftMaxHeapSize=512m -Xmx31g jvm args. With the fix, only 18 times in 20 sec.
GHA passed.

This change also improved gc logging:

Before:

[6.831s][info][gc          ] Trigger: Free (52230K) is below minimum threshold (52428K)
[6.831s][info][gc,free     ] Free: 1587M, Max: 1024K regular, 1539M humongous, Frag: 2% 
external, 18% internal; Used: 352M, Mutator Free: 1940 Collector Reserve: 103M, Max: 1024K; Used: 0B

After:

[8.358s][info][gc          ] Trigger: Free (Soft mutator free) (51498K) is below minimum threshold (52428K)
[8.358s][info][gc,free     ] Whole heap stats: Total free: 1509M, Total used: 401M, Max free in a single region: 
1024K, Max humongous: 1490M; Frag stats: External: 0%, Internal: 21%;  Mutator freeset stats: Partition count: 
1911, Reserved: 1509M, Max free available in a single region: 1024K; Collector freeset stats: Partition count: 
122, Reserved: 102M, Max free available in a single region: 1024K;

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-8372543: Shenandoah: undercalculated the available size when soft max takes effect (Bug - P4)

Reviewers

William Kemper (@earthling-amzn - Reviewer)
Kelvin Nilsen (@kdnilsen - Committer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 28622

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

Using diff file

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

Using Webrev

Link to Webrev Comment

bridgekeeper · 2025-12-03T02:03:37Z

👋 Welcome back rgithubli! 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 · 2025-12-03T02:05:13Z

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

8372543: Shenandoah: undercalculated the available size when soft max takes effect

Reviewed-by: wkemper, kdnilsen

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

3f33eaa: 8373649: Convert simple AtomicAccess usage in ConcurrentHashTable to use Atomic
1748737: 8372988: Test runtime/Nestmates/membership/TestNestHostErrorWithMultiThread.java failed: Unexpected interrupt
317788f: 8360160: ubuntu-22-04 machine is failing client tests
... and 204 more: https://git.openjdk.org/jdk/compare/5627ff2d9165ee1f7354c1ff1626f4949ef7fa3f...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 (@earthling-amzn, @kdnilsen) 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 · 2025-12-03T02:06:22Z

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

hotspot-gc
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.

… takes effect

mlbridge · 2025-12-03T07:31:41Z

Webrevs

earthling-amzn

A few nits. Thank you for adding a test case for this!

src/hotspot/share/gc/shenandoah/heuristics/shenandoahAdaptiveHeuristics.cpp

earthling-amzn · 2025-12-03T18:44:37Z

src/hotspot/share/gc/shenandoah/heuristics/shenandoahAdaptiveHeuristics.cpp

  size_t min_threshold = min_free_threshold();
  if (available < min_threshold) {
-    log_trigger("Free (%zu%s) is below minimum threshold (%zu%s)",
+    log_trigger("Free (Soft mutator free) (%zu%s) is below minimum threshold (%zu%s)",


Changing this will break some log parsers, do we really need this?

Talked offline. Free is overloaded in logs. Sometimes it means soft free, sometimes it means total free. Make it as Free (Soft) here.

src/hotspot/share/gc/shenandoah/heuristics/shenandoahCompactHeuristics.cpp

src/hotspot/share/gc/shenandoah/shenandoahFreeSet.cpp

src/hotspot/share/gc/shenandoah/shenandoahFreeSet.hpp

earthling-amzn · 2025-12-04T20:30:27Z

src/hotspot/share/gc/shenandoah/shenandoahGlobalGeneration.cpp

-  assert(max_capacity() >= ShenandoahHeap::heap()->soft_max_capacity(), "Max capacity must be greater than soft max capacity.");
-  size_t soft_tail = max_capacity() - ShenandoahHeap::heap()->soft_max_capacity();
-  return (available > soft_tail) ? (available - soft_tail) : 0;
+size_t ShenandoahGlobalGeneration::soft_available_exclude_evac_reserve() const {


Two questions:

How is this override different from the default implementation now?

Should we not also take the minimum of this value and free_set()->available() as we do elsewhere?

Good call. No functional differences except for a safety assert: assert(max_capacity() >= soft_max, which isn't that necessary since the app wouldn't start if this wasn't true: code. Will remove the override.

I don't think it's needed for global. free_set()->available() (space reserved for mutators) could be smaller than mutator_soft_max when there's an old gen taking space. If there's no generation, the mutator_soft_max should always be less or equal than free_set()->available().

kdnilsen · 2025-12-05T19:34:12Z

src/hotspot/share/gc/shenandoah/shenandoahGeneration.cpp

-size_t ShenandoahGeneration::soft_available() const {
-  size_t result = available(ShenandoahHeap::heap()->soft_max_capacity());
+size_t ShenandoahGeneration::soft_available_exclude_evac_reserve() const {
+  size_t result = available(ShenandoahHeap::heap()->soft_max_capacity() * (100.0 - ShenandoahEvacReserve) / 100);


I'm a little uncomfortable with this approach. It's mostly a question of how we name it. The evac reserve is not always this value. In particular, we may shrink the young evac reserves after we have selected the cset. Also of concern is that if someone invokes this function on old_generation(), it looks like they'll get a bogus (not meaningful) value.

I think I'd be more comfortable with naming this to something like "mutator_available_when_gc_is_idle()". If we keep it virtual, then OldGeneration should override with "assert(false, "Not relevant to old generation")

Talked offline. Rename this to soft_mutator_available

rgithubli · 2025-12-09T00:01:21Z

src/hotspot/share/gc/shenandoah/heuristics/shenandoahAdaptiveHeuristics.cpp

  size_t min_threshold = min_free_threshold();
  if (available < min_threshold) {
-    log_trigger("Free (%zu%s) is below minimum threshold (%zu%s)",
+    log_trigger("Free (Soft) (%zu%s) is below minimum threshold (%zu%s)",


Forgot to use log format macros PROPERFMT & PROPERFMTARGS here. Will update.

kdnilsen · 2025-12-15T19:55:14Z

src/hotspot/share/gc/shenandoah/shenandoahGeneration.cpp

-size_t ShenandoahGeneration::soft_available() const {
-  size_t result = available(ShenandoahHeap::heap()->soft_max_capacity());
+size_t ShenandoahGeneration::soft_available_exclude_evac_reserve() const {
+  size_t result = available(ShenandoahHeap::heap()->soft_max_capacity() * (100.0 - ShenandoahEvacReserve) / 100);


earthling-amzn

Looks good!

rgithubli · 2025-12-16T04:12:16Z

/integrate

openjdk · 2025-12-16T04:14:07Z

@rgithubli
Your change (at version 8dea516) is now ready to be sponsored by a Committer.

pengxiaolong · 2025-12-16T07:01:15Z

Looks good, thanks!

/sponsor

openjdk · 2025-12-16T07:02:20Z

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

3f33eaa: 8373649: Convert simple AtomicAccess usage in ConcurrentHashTable to use Atomic
1748737: 8372988: Test runtime/Nestmates/membership/TestNestHostErrorWithMultiThread.java failed: Unexpected interrupt
317788f: 8360160: ubuntu-22-04 machine is failing client tests
... and 204 more: https://git.openjdk.org/jdk/compare/5627ff2d9165ee1f7354c1ff1626f4949ef7fa3f...master

Your commit was automatically rebased without conflicts.

openjdk · 2025-12-16T07:02:34Z

@pengxiaolong @rgithubli Pushed as commit b1e8c4e.

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

openjdk bot added hotspot-gc hotspot-gc-dev@openjdk.org shenandoah shenandoah-dev@openjdk.org labels Dec 3, 2025

8372543: Shenandoah: undercalculated the available size when soft max…

b23e9ff

… takes effect

rgithubli force-pushed the 8372543 branch from 61a66f9 to b23e9ff Compare December 3, 2025 07:23

rgithubli marked this pull request as ready for review December 3, 2025 07:27

openjdk bot added the rfr Pull request is ready for review label Dec 3, 2025

earthling-amzn suggested changes Dec 3, 2025

View reviewed changes

rgithubli added 3 commits December 4, 2025 00:16

log format fixes

24848c9

Rename soft_available. Change Generation soft avail impl

103ce8f

Remove unused freeset includes

599cc2d

rgithubli requested a review from earthling-amzn December 4, 2025 18:48

earthling-amzn reviewed Dec 4, 2025

View reviewed changes

kdnilsen suggested changes Dec 5, 2025

View reviewed changes

rgithubli mentioned this pull request Dec 8, 2025

8372543: Shenandoah: undercalculated the available size when soft max takes effect corretto/corretto-17#256

Merged

rgithubli commented Dec 9, 2025

View reviewed changes

log and naming fixes

8dea516

rgithubli requested review from earthling-amzn and kdnilsen December 11, 2025 22:09

kdnilsen approved these changes Dec 15, 2025

View reviewed changes

earthling-amzn approved these changes Dec 16, 2025

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Dec 16, 2025

openjdk bot added the sponsor Pull request is ready to be sponsored label Dec 16, 2025

openjdk bot added the integrated Pull request has been integrated label Dec 16, 2025

openjdk bot closed this Dec 16, 2025

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 Dec 16, 2025

Conversation

rgithubli commented Dec 3, 2025 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewers

Reviewing

Uh oh!

bridgekeeper bot commented Dec 3, 2025

Uh oh!

openjdk bot commented Dec 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openjdk bot commented Dec 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mlbridge bot commented Dec 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

earthling-amzn left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

earthling-amzn left a comment

Choose a reason for hiding this comment

Uh oh!

rgithubli commented Dec 16, 2025

Uh oh!

openjdk bot commented Dec 16, 2025

Uh oh!

pengxiaolong commented Dec 16, 2025

Uh oh!

openjdk bot commented Dec 16, 2025

Uh oh!

openjdk bot commented Dec 16, 2025

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

rgithubli commented Dec 3, 2025 •

edited by openjdk bot

Loading

openjdk bot commented Dec 3, 2025 •

edited

Loading

openjdk bot commented Dec 3, 2025 •

edited

Loading

mlbridge bot commented Dec 3, 2025 •

edited

Loading