RFR: 8357445: G1: Time-Based Heap Uncommit During Idle Periods [v4]

[Thomas Schatzl]

On Thu, 17 Jul 2025 07:17:49 GMT, Thomas Schatzl <tschatzl at openjdk.org> wrote:

>> src/hotspot/share/gc/g1/g1_globals.hpp line 391:
>> 
>>> 389:           "attempt to uncommit memory")                                     \
>>> 390:           range(1, max_uintx)                                               \
>>> 391:                                                                             \
>> 
>> Just an initial drive-by comment:
>> * not sure I would make `G1UseTimeBasedHeapSizing` experimental (_maybe_ G1MinRegionsToUncommit`, but it seems very similar in use), and disabled by default. I would enable it by default, and make it diagnostic. While the difference between experimental and diagnostic is somewhat arguable, the former is more for developers to enable/disable, and the latter for support team to handle support issues (i.e. diagnose them).
>> As for the default value, I expect this to be beneficial always, and the default value indicates a long enough default delay for doing that, i.e. the overhead should be negligible.
>> * new manageble flags need a CSR to officially announce the functionality (and the feature needs a release note). Probably also tuning guide changes, but since it is not editable by the public, we at Oracle will handle this.
>> I am not yet completely convinced to make these manageable, although I can imagine some reasons. Can you give a good use case you specifically encountered to do so?
>
> Release-note howto: https://openjdk.org/guide/index.html#release-notes. For now I tagged the issue with `release-note=yes`. Note that it can be written later.

One reason why I do not see the need for these flags to be manageable is that this heap shrinking is a supportive function if the application is "idle". Main heap sizing should be done by the existing heap sizing/AHS at GC events.

>From the CR text:
>  Doing heap sizing based on garbage collections has a big disadvantage: if there are no garbage collections (due to no application activity) there is a risk that a large amount of heap is kept committed unnecessarily *for a long time*. 
(emphasis mine)

If we are talking about "a long time", so there does not seem to be a need for changing it during runtime (or change it at all). It should not matter if that "long time" is "long time +- small epsilon", and so, allowing dynamic change of "a long time" to another "long time" seems unnecessary without a very good use case.

Please consider not necessarily the current situation, but with "full" AHS.

Another question is whether you had thoughts about the interaction with (JDK-8213198)[https://bugs.openjdk.org/browse/JDK-8213198], as this change seems to be a subset of the other. (That's just curiosity from me, I think this feature is useful as is, and if the other ever materializes, we can always reconsider).

Otoh decreasing the heap by this mechanism will eventually trigger a marking.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/26240#discussion_r2212529861