8238687: Investigate memory uncommit during young collections in G1 #25832

walulyai · 2025-06-16T14:05:52Z

Hi all,

Please review this change to the G1 heap resizing policy, aimed at improving alignment with the configured GCTimeRatio. The GCTimeRatio is intended to manage the balance between GC time and Application execution time. G1's current implementation of GCTimeRatio appears to have drifted from its intended purpose over time. Therefore, we need to change G1’s use of the GCTimeRatio to better manage heap sizes without relying on additional magic constants.

The primary goal is to enable both heap expansion and shrinking at the end of any GC, rather than limiting shrinking to only the Remark or Full GC pauses as is currently done. We achieve this using heuristics that monitor both short-term and long-term GC time ratios relative to the configured GCTimeRatio.

The short-term policy adjusts a counter based on whether recent GC time is above or below a target range around GCTimeRatio (as defined by G1MinimumPercentOfGCTimeRatio). When the counter crosses predefined thresholds, the heap may be expanded or shrunk accordingly.
The long-term policy evaluates the GC time ratio over a long-term interval and triggers resizing if the number of recorded ratios exceeds a threshold and the GC time ratio over the long-term interval is outside the target range.
These heuristics allow for responsive heap resizing (both expansion and shrinking) at the end of any GC, guided by actual GC performance rather than fixed thresholds or constants.

We are increasing the default GCTimeRatio from 12 to 24, since under the new policy, the current default leads to overly aggressive heap shrinking as the GCTimeRatio allows for a lot more GC overhead.

Additionally, we are removing the heap resizing step at the end of the Remark pause which was based on MinHeapFreeRatio and MaxHeapFreeRatio. We keep this MinHeapFreeRatio-MaxHeapFreeRatio based resizing logic at the end of Full GC and Remark pauses that may have been triggered by PeriodicGCs.

As a result of these changes, some applications may settle at more appropriate and in some cases smaller heap sizes for the configured GCTimeRatio. While this may appear as a regression in some benchmarks that are sensitive to heap size, it represents more accurate G1 behavior with respect to the GCTimeRatio. Although smaller heap sizes may lead to more frequent GCs, this is the expected outcome, provided the cumulative GC overhead remains within the limits defined by the GCTimeRatio.

Testing: Mach5 Tier 1-7

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

Issues

JDK-8238687: Investigate memory uncommit during young collections in G1 (Enhancement - P4)
JDK-8247843: Reconsider G1 default GCTimeRatio value (Enhancement - P4)
JDK-8248324: G1: Remove resizing during Remark (Enhancement - P4)

Contributors

Thomas Schatzl <[email protected]>

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 25832

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

Using diff file

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

Using Webrev

Link to Webrev Comment

walulyai · 2025-06-16T14:06:08Z

/contributor add tschatzl

bridgekeeper · 2025-06-16T14:06:52Z

👋 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 · 2025-06-16T14:07:19Z

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

openjdk · 2025-06-16T14:08:06Z

@walulyai
Contributor Thomas Schatzl <[email protected]> successfully added.

walulyai · 2025-06-16T14:08:53Z

/issue add JDK-8247843

openjdk · 2025-06-16T14:08:57Z

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

hotspot

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 · 2025-06-16T14:09:37Z

@walulyai
Adding additional issue to issue list: 8247843: Reconsider G1 default GCTimeRatio value.

walulyai · 2025-06-16T14:10:17Z

/issue add JDK-8248324

openjdk · 2025-06-16T14:10:21Z

@walulyai
Adding additional issue to issue list: 8248324: G1: Remove resizing during Remark.

mlbridge · 2025-06-16T14:36:04Z

Webrevs

walulyai · 2025-06-16T14:47:52Z

/label add hotspot-gc

openjdk · 2025-06-16T14:48:40Z

@walulyai
The hotspot-gc label was successfully added.

openjdk · 2025-06-17T13:40:44Z

@walulyai 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 G1HeapResizePolicyV2
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

albertnetymk · 2025-06-18T11:21:07Z

src/hotspot/share/gc/g1/g1_globals.hpp

+  product(size_t, G1ShrinkByPercentOfAvailable, 50, EXPERIMENTAL,           \
+          "When shrinking, maximum % of free space to claim.")              \
+          range(0, 100)                                                     \
+                                                                            \
+  product(size_t, G1MinimumPercentOfGCTimeRatio, 25, EXPERIMENTAL,          \
+          "Percentage of GCTimeRatio G1 will try to avoid going below.")    \
+          range(0, 100)                                                     \


If they are in [0,100], maybe uint is enough?

albertnetymk · 2025-06-18T11:22:53Z

src/hotspot/share/utilities/numberSeq.hpp

@@ -47,7 +47,7 @@ class AbsSeq: public CHeapObj<mtInternal> {
  void init(double alpha);

 protected:
-  int    _num; // the number of elements in the sequence
+  uint    _num; // the number of elements in the sequence


All int -> uint changes in this class/file are good; can they be extracted out to its own PR/ticket?

albertnetymk · 2025-06-18T11:35:53Z

src/hotspot/share/gc/g1/g1CollectedHeap.cpp

+    expand(resize_bytes, _workers);
+  } else {
+    shrink(resize_bytes);
+    uncommit_regions_if_necessary();


I wonder if it's more symmetric if this uncommit logic is inlined to shrink.

albertnetymk · 2025-06-18T11:43:01Z

src/hotspot/share/utilities/numberSeq.hpp

@@ -127,6 +128,8 @@ class TruncatedSeq: public AbsSeq {
  double oldest() const; // the oldest valid value in the sequence
  double predict_next() const; // prediction based on linear regression

+  virtual void reset();
+  bool is_full() const { return _length == _num; }


Seems unused.

albertnetymk · 2025-06-18T11:46:59Z

src/hotspot/share/utilities/numberSeq.cpp

  AbsSeq(alpha), _length(length), _next(0) {
  _sequence = NEW_C_HEAP_ARRAY(double, _length, mtInternal);
-  for (int i = 0; i < _length; ++i)
-    _sequence[i] = 0.0;
+  TruncatedSeq::reset();


Not sure why a new method (reset) is needed inside this PR.

clean init

6573042

openjdk bot added the hotspot [email protected] label Jun 16, 2025

walulyai marked this pull request as ready for review June 16, 2025 14:32

openjdk bot added the rfr Pull request is ready for review label Jun 16, 2025

openjdk bot added the hotspot-gc [email protected] label Jun 16, 2025

remove unrequired changes - kim

81a1e18

openjdk bot added the merge-conflict Pull request has merge conflict with target branch label Jun 17, 2025

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

2d5b7cd

openjdk bot removed the merge-conflict Pull request has merge conflict with target branch label Jun 17, 2025

albertnetymk reviewed Jun 18, 2025

View reviewed changes

Albert suggestions

df4f7ce

8238687: Investigate memory uncommit during young collections in G1 #25832

Are you sure you want to change the base?

8238687: Investigate memory uncommit during young collections in G1 #25832

Conversation

walulyai commented Jun 16, 2025 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issues

Contributors

Reviewing

Uh oh!

walulyai commented Jun 16, 2025

Uh oh!

bridgekeeper bot commented Jun 16, 2025

Uh oh!

openjdk bot commented Jun 16, 2025

Uh oh!

openjdk bot commented Jun 16, 2025

Uh oh!

walulyai commented Jun 16, 2025

Uh oh!

openjdk bot commented Jun 16, 2025

Uh oh!

openjdk bot commented Jun 16, 2025

Uh oh!

walulyai commented Jun 16, 2025

Uh oh!

openjdk bot commented Jun 16, 2025

Uh oh!

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

Uh oh!

Webrevs

Uh oh!

walulyai commented Jun 16, 2025

Uh oh!

openjdk bot commented Jun 16, 2025

Uh oh!

openjdk bot commented Jun 17, 2025

Uh oh!

albertnetymk Jun 18, 2025

Choose a reason for hiding this comment

Uh oh!

albertnetymk Jun 18, 2025

Choose a reason for hiding this comment

Uh oh!

albertnetymk Jun 18, 2025

Choose a reason for hiding this comment

Uh oh!

albertnetymk Jun 18, 2025

Choose a reason for hiding this comment

Uh oh!

albertnetymk Jun 18, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

walulyai commented Jun 16, 2025 •

edited by openjdk bot

Loading

mlbridge bot commented Jun 16, 2025 •

edited

Loading