From c73c9202038d88a9c56be39f445d5177709170f9 Mon Sep 17 00:00:00 2001 From: Thomas Schatzl Date: Wed, 12 Feb 2025 11:48:56 +0100 Subject: [PATCH 1/4] 8349906 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. Testing: gha, tier1-7 (with other changes) Hth, Thomas --- src/hotspot/share/gc/g1/g1SurvRateGroup.cpp | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/hotspot/share/gc/g1/g1SurvRateGroup.cpp b/src/hotspot/share/gc/g1/g1SurvRateGroup.cpp index 1eaaf44a1fbde..2cd340b26754e 100644 --- a/src/hotspot/share/gc/g1/g1SurvRateGroup.cpp +++ b/src/hotspot/share/gc/g1/g1SurvRateGroup.cpp @@ -68,13 +68,21 @@ void G1SurvRateGroup::stop_adding_regions() { _accum_surv_rate_pred = REALLOC_C_HEAP_ARRAY(double, _accum_surv_rate_pred, _num_added_regions, mtGC); _surv_rate_predictors = REALLOC_C_HEAP_ARRAY(TruncatedSeq*, _surv_rate_predictors, _num_added_regions, mtGC); + // Assume that the prediction for the newly added regions is the same as the + // ones at the (current) end of the array. Particularly predictions at the end + // of this array fairly seldom get updated, so having a better initial value + // that is at least somewhat related to the actual application is preferable. + double new_pred = _stats_arrays_length > 1 + ? _accum_surv_rate_pred[_stats_arrays_length - 1] - _accum_surv_rate_pred[_stats_arrays_length - 2] + : InitialSurvivorRate; + for (size_t i = _stats_arrays_length; i < _num_added_regions; ++i) { // Initialize predictors and accumulated survivor rate predictions. _surv_rate_predictors[i] = new TruncatedSeq(10); - _surv_rate_predictors[i]->add(InitialSurvivorRate); - _accum_surv_rate_pred[i] = ((i == 0) ? 0.0 : _accum_surv_rate_pred[i-1]) + InitialSurvivorRate; + _surv_rate_predictors[i]->add(new_pred); + _accum_surv_rate_pred[i] = ((i == 0) ? 0.0 : _accum_surv_rate_pred[i-1]) + new_pred; } - _last_pred = InitialSurvivorRate; + _last_pred = new_pred; _stats_arrays_length = _num_added_regions; } @@ -110,7 +118,7 @@ double G1SurvRateGroup::accum_surv_rate_pred(uint age) const { void G1SurvRateGroup::fill_in_last_surv_rates() { if (_num_added_regions > 0) { // conservative double surv_rate = _surv_rate_predictors[_num_added_regions-1]->last(); - for (size_t i = _num_added_regions; i < _stats_arrays_length; ++i) { + for (uint i = _num_added_regions; i < _stats_arrays_length; ++i) { _surv_rate_predictors[i]->add(surv_rate); } } From 5c4ded01c1611a0c9f612bc2f9a565c547a6585a Mon Sep 17 00:00:00 2001 From: Thomas Schatzl Date: Wed, 12 Feb 2025 12:32:49 +0100 Subject: [PATCH 2/4] * remove whitespace --- src/hotspot/share/gc/g1/g1SurvRateGroup.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hotspot/share/gc/g1/g1SurvRateGroup.cpp b/src/hotspot/share/gc/g1/g1SurvRateGroup.cpp index 2cd340b26754e..39fc75430466a 100644 --- a/src/hotspot/share/gc/g1/g1SurvRateGroup.cpp +++ b/src/hotspot/share/gc/g1/g1SurvRateGroup.cpp @@ -75,7 +75,7 @@ void G1SurvRateGroup::stop_adding_regions() { double new_pred = _stats_arrays_length > 1 ? _accum_surv_rate_pred[_stats_arrays_length - 1] - _accum_surv_rate_pred[_stats_arrays_length - 2] : InitialSurvivorRate; - + for (size_t i = _stats_arrays_length; i < _num_added_regions; ++i) { // Initialize predictors and accumulated survivor rate predictions. _surv_rate_predictors[i] = new TruncatedSeq(10); From a09bc25e4a76cf657ee78b39d3ea2a54e198e255 Mon Sep 17 00:00:00 2001 From: Thomas Schatzl Date: Tue, 25 Feb 2025 17:40:54 +0100 Subject: [PATCH 3/4] * kbarrett review: do not change the type of loop variable * ayang review: use actual last value instead of prediction for newly allocated survivor rate groups --- src/hotspot/share/gc/g1/g1SurvRateGroup.cpp | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/hotspot/share/gc/g1/g1SurvRateGroup.cpp b/src/hotspot/share/gc/g1/g1SurvRateGroup.cpp index 39fc75430466a..5856d4c4fd641 100644 --- a/src/hotspot/share/gc/g1/g1SurvRateGroup.cpp +++ b/src/hotspot/share/gc/g1/g1SurvRateGroup.cpp @@ -28,6 +28,8 @@ #include "logging/log.hpp" #include "memory/allocation.hpp" +#include "logging/logStream.hpp" + G1SurvRateGroup::G1SurvRateGroup() : _stats_arrays_length(0), _num_added_regions(0), @@ -79,8 +81,13 @@ void G1SurvRateGroup::stop_adding_regions() { for (size_t i = _stats_arrays_length; i < _num_added_regions; ++i) { // Initialize predictors and accumulated survivor rate predictions. _surv_rate_predictors[i] = new TruncatedSeq(10); - _surv_rate_predictors[i]->add(new_pred); - _accum_surv_rate_pred[i] = ((i == 0) ? 0.0 : _accum_surv_rate_pred[i-1]) + new_pred; + if (i == 0) { + _surv_rate_predictors[i]->add(InitialSurvivorRate); + _accum_surv_rate_pred[i] = 0.0; + } else { + _surv_rate_predictors[i]->add(_surv_rate_predictors[i-1]->last()); + _accum_surv_rate_pred[i] = _accum_surv_rate_pred[i-1] + new_pred; + } } _last_pred = new_pred; @@ -118,7 +125,7 @@ double G1SurvRateGroup::accum_surv_rate_pred(uint age) const { void G1SurvRateGroup::fill_in_last_surv_rates() { if (_num_added_regions > 0) { // conservative double surv_rate = _surv_rate_predictors[_num_added_regions-1]->last(); - for (uint i = _num_added_regions; i < _stats_arrays_length; ++i) { + for (size_t i = _num_added_regions; i < _stats_arrays_length; ++i) { _surv_rate_predictors[i]->add(surv_rate); } } From fc2dde0cfee3592795ece7b84175eb6533501dcc Mon Sep 17 00:00:00 2001 From: Thomas Schatzl Date: Wed, 26 Feb 2025 09:26:56 +0100 Subject: [PATCH 4/4] * kbarrett review - remove include previously used for debugging --- src/hotspot/share/gc/g1/g1SurvRateGroup.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/hotspot/share/gc/g1/g1SurvRateGroup.cpp b/src/hotspot/share/gc/g1/g1SurvRateGroup.cpp index 5856d4c4fd641..c5c0ca992b078 100644 --- a/src/hotspot/share/gc/g1/g1SurvRateGroup.cpp +++ b/src/hotspot/share/gc/g1/g1SurvRateGroup.cpp @@ -28,8 +28,6 @@ #include "logging/log.hpp" #include "memory/allocation.hpp" -#include "logging/logStream.hpp" - G1SurvRateGroup::G1SurvRateGroup() : _stats_arrays_length(0), _num_added_regions(0),