RFR: 8373225: GenShen: More adaptive old-generation growth heuristics [v5]

Thu Dec 11 00:58:31 UTC 2025

On Wed, 10 Dec 2025 21:50:29 GMT, Kelvin Nilsen <kdnilsen at openjdk.org> wrote:

>> When old-gen consumes a small percentage of heap size, trigger when old-gen expands by more than ShenandoahMinOldGenGrowthPercent, with default value 50%, from the live data in old at time of previous old-gen mark.
>> 
>> When old-gen consumes a larger percentage of heap size, we trigger when old-gen expands by more than  ShenandoahMinOldGenGrowthRemainingHeapPercent, with default value 35%, of the memory not live in old at the last marking of old.
>
> Kelvin Nilsen has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Remove FRACTIONAL_DENOMINATOR constat

I left a few cosmetic comments, but I have the feeling that the verbal description of the parameters isn't faithful to the actual implementation.

Moreover, it appeared to me, quite subjectively, that the triggering criteria are sufficiently complex that capturing them accurately as tunables that users could usefully tune is a bit difficult without reference to how the adjustments themselves work.

I think your changes are probably good and yield improvements in performance that you can demonstrate in practice. So I think these changes should go in.

That said, I feel there may be an opportunity to slightly simplify the implementation or at least expose only those tunables that one can expect to be able to adjust easily. As it stands, I find the implementation to be such that most users will likely have a hard time using these tunables in any intelligent way. I'd like us to see if we can somehow convey the tuning aspect more clearly in the description of these parameters (while hopefully not compormising accuracy of the descriptions).

src/hotspot/share/gc/shenandoah/heuristics/shenandoahOldHeuristics.hpp line 105:

> 103:   size_t _fragmentation_last_old_region;
> 104: 
> 105:   // adapted value of ShenandoahOldGarbageThreshold

May be reword to:

// a dynamic threshold of garbage for an old
// region to be deemed eligible for evacuation.

since `ShenandoahOldGarbageThreshold` is a constant parameter to the JVM.

src/hotspot/share/gc/shenandoah/heuristics/shenandoahOldHeuristics.hpp line 206:

> 204:   bool is_experimental() override;
> 205: 
> 206: 

Although just an accessor, I'd document this API, perhaps using its intended usage as understood by its client:

// Returns the current value of a dynamically
// adjusted threshold percentage of garbage
// above which an Old region should be deemed
// eligible for evacuation.

src/hotspot/share/gc/shenandoah/heuristics/shenandoahOldHeuristics.hpp line 214:

> 212: 
> 213:   // The normal old_garbage_threshold is specified by ShenandoahOldGarbageThreshold command-line argument, with default
> 214:   // value 25, denoting that a region that has at least 25% garbage is eligible for compaction.  With default values for

compaction or evacuation?

src/hotspot/share/gc/shenandoah/shenandoahGeneration.cpp line 509:

> 507: 
> 508: // Preselect for inclusion into the collection set regions whose age is at or above tenure age which contain more than
> 509: // the old garbage threshold amount of garbage.  We identify these regions by setting the appropriate entry of

Is amount in this sentence a percentage? If so, I'd say: `We only select regions whose garbage percentage exceeds a dynamically adjusted threshold.`
Using "old garbage threshold amount" by itself can be confusing, since "amount" doesn't sound like a percentage which is what I believe we mean here.

src/hotspot/share/gc/shenandoah/shenandoahOldGeneration.cpp line 119:

> 117:     _card_scan(nullptr),
> 118:     _state(WAITING_FOR_BOOTSTRAP),
> 119:     _growth_percent_before_compaction(INITIAL_PERCENT_GROWTH_BEFORE_COMPACTION)

Use either "percent_growth" or "growth_percent" consistently in both names.

src/hotspot/share/gc/shenandoah/shenandoahOldGeneration.hpp line 306:

> 304: 
> 305:   // How much growth in usage before we trigger old collection as a percent of soft_max_capacity
> 306:   size_t _growth_percent_before_compaction;

I'd prefer we didn't use the term "compaction". Would replacing it with "collection", as in the comment, work? Or is there a specific reason you want to say "compaction" here?

src/hotspot/share/gc/shenandoah/shenandoahOldGeneration.hpp line 307:

> 305:   // How much growth in usage before we trigger old collection as a percent of soft_max_capacity
> 306:   size_t _growth_percent_before_compaction;
> 307: 

See comment on naming of percent quantities in .cpp above.

src/hotspot/share/gc/shenandoah/shenandoah_globals.hpp line 65:

> 63:           "(Generational mode only) If the usage within old generation "    \
> 64:           "has grown by at least this percent of its live memory size "     \
> 65:           "at the start of the previous old-generation marking effort, "    \

Did you intend to say "at the _end_ of the previous old-generation marking effort" above?

src/hotspot/share/gc/shenandoah/shenandoah_globals.hpp line 71:

> 69:   product(double, ShenandoahMinOldGenGrowthRemainingHeapPercent,            \
> 70:           35, EXPERIMENTAL,                                                 \
> 71:           "(Generational mode only) If the usage within old generation "    \

I find this comment very confusing, and not amenable to use as a tuning device. Can thus be converted into a knob that is history-independent/memory-less -- i.e. solely state-dependent?

Otherwise, let's try and word this so that it's simpler to parse. This could include a multiple sentence description.

src/hotspot/share/gc/shenandoah/shenandoah_globals.hpp line 90:

> 88:           "ShenandoahGenerationalDoNotIgnoreGrowthAfterYoungCycles "        \
> 89:           "consecutive cycles have been completed following the "           \
> 90:           "preceding old-gen collection.")                                  \

Here again, like my remark below, we can effectively decouple the two options and simplify the verbage by merely saying:

// Do not use Old generation growth as a triggering criterion
// when usage is lower than this percentage of heap.

I am not sure if "of heap" is correct, or if there is some other implicit percentage of the old generation capacity that one has in mind here.

src/hotspot/share/gc/shenandoah/shenandoah_globals.hpp line 96:

> 94:           100, EXPERIMENTAL,                                                \
> 95:           "(Generational mode only) Even if the usage of old generation "   \
> 96:           "is below ShenandoahIgnoreOldGrowthBelowPercentage, "             \

The reference to `ShenandoahIgnoreOldGrowthBelowPercentage` (SIOGBP) seems to me to be spurious and confusing. I think this might be a simpler phrasing, without any reference to SIOGBP:

\\ Trigger an Old collection if Old generation usage has grown,
\\ and this many Young collections have happened,
\\ since the last Old collection.

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

Marked as reviewed by ysr (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/28561#pullrequestreview-3564758554
PR Review Comment: https://git.openjdk.org/jdk/pull/28561#discussion_r2608434949
PR Review Comment: https://git.openjdk.org/jdk/pull/28561#discussion_r2608566349
PR Review Comment: https://git.openjdk.org/jdk/pull/28561#discussion_r2608443529
PR Review Comment: https://git.openjdk.org/jdk/pull/28561#discussion_r2608509748
PR Review Comment: https://git.openjdk.org/jdk/pull/28561#discussion_r2608643499
PR Review Comment: https://git.openjdk.org/jdk/pull/28561#discussion_r2608604393
PR Review Comment: https://git.openjdk.org/jdk/pull/28561#discussion_r2608648317
PR Review Comment: https://git.openjdk.org/jdk/pull/28561#discussion_r2608454990
PR Review Comment: https://git.openjdk.org/jdk/pull/28561#discussion_r2608498445
PR Review Comment: https://git.openjdk.org/jdk/pull/28561#discussion_r2608491539
PR Review Comment: https://git.openjdk.org/jdk/pull/28561#discussion_r2608477270