RFR: 8349906: G1: Improve initial survivor rate for newly used young regions
Kim Barrett
kbarrett at openjdk.org
Mon Feb 24 13:02:54 UTC 2025
On Wed, 12 Feb 2025 10:55:46 GMT, Thomas Schatzl <tschatzl at openjdk.org> wrote:
> Hi all,
>
> please review this change that tries to improve the survivor rate initial values for newly expanded regions.
>
> Currently G1 uses `InitialSurvivorRate` as survivor rate for such regions, but it is typically a pretty bad choice because
>
> * it's rather conservative, estimating that 40% of region contents will survive
> * such a conservative value is kind of bad particularly in cases for regions that are expanded late in the mutator phase because they are not frequently updated (and with our running weights changes get propagated over a very long time), i.e. this 40% sticks for a long time (*)
> * it is a random value, i.e. not particularly specific to the application.
>
> The suggestion is to use the survivor rate for the last region we know the survivor rate already.
>
> (*) to clarify this a little: G1 keeps track of `[0...m]` survivor rate predictors. For a given garbage collection, `[0...n]` of those are updated (`n` is the number of eden/survivor regions depending on the rate group). However those for `]n...m]` are not, particularly those in that range that are seldom allocated, the predictors are not updated very frequently. Now the young gen sizing uses these predictions "at the end" of the predictor anyway, and since they are infrequently updated and their values are very conservative, G1 won't expand young gen as much as it could/should.
>
> Testing: gha, tier1-7 (with other changes)
>
> Hth,
> Thomas
Changes requested by kbarrett (Reviewer).
src/hotspot/share/gc/g1/g1SurvRateGroup.cpp line 79:
> 77: : InitialSurvivorRate;
> 78:
> 79: for (size_t i = _stats_arrays_length; i < _num_added_regions; ++i) {
Shouldn't this iteration variable be similarly updated as was done in fill_in_last_surv_rates, to avoid
some implicit narrowing? Similarly for the loop on line 49 in the destructor? Basically, I'm suggesting
doing all or none of these in this PR (with maybe a preference for none, and do a separate sweep).
-------------
PR Review: https://git.openjdk.org/jdk/pull/23584#pullrequestreview-2637013120
PR Review Comment: https://git.openjdk.org/jdk/pull/23584#discussion_r1967598618
More information about the hotspot-gc-dev
mailing list