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

[Y. Srinivas Ramakrishna]

On Wed, 10 Dec 2025 22:28:01 GMT, Y. Srinivas Ramakrishna <ysr at openjdk.org> wrote:

>> Kelvin Nilsen has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Remove FRACTIONAL_DENOMINATOR constat
>
> 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.

As I write this, I realize "Old region" may not be the right term here. It should be "an Old or otherwise tenurable region" because it seems regions that are Young but tenurable are filtered through this check, not just Old regions?

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

And make those two consistent with either "percent_live" or "live_percent" below.

(These comments from me actually belong in the .hpp where these are defined.)

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

I might even combine the descriptions of the previous and this parameter, which work in tandem, to describe how to tune them. e.g.


Old generation collection is triggered by determining when its usage has grown past a threshold since the end of the last Old generation collection, viz.
1. if the usage exceeds the amount considered live at the last old marking cycle plus ShenandoahMinOldGenGrowthPercent markup, or
2. if the current remaining headroom falls below ShenandoahMinOldGenGrowthRemainingHeapPercent of the complement of what was considered live at the last old marking cycle.


I am not sure if the verbage in (2) above is correct.

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

As I read the code for the old heuristic growth trigger, I realize my rewording above is incorrect.

I think the code in the triggering could be simplified a bit to allow a more crisp description of these parameters.

Let me talk with you offline (face to face).

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

PR Review Comment: https://git.openjdk.org/jdk/pull/28561#discussion_r2608586522
PR Review Comment: https://git.openjdk.org/jdk/pull/28561#discussion_r2608646531
PR Review Comment: https://git.openjdk.org/jdk/pull/28561#discussion_r2608735072
PR Review Comment: https://git.openjdk.org/jdk/pull/28561#discussion_r2608743619