RFR: 8294850: Make rs length/pending card predictors dependent on gc phase

Albert Mingkun Yang ayang at openjdk.org
Wed Oct 12 09:36:05 UTC 2022


On Tue, 11 Oct 2022 12:53:29 GMT, Thomas Schatzl <tschatzl at openjdk.org> wrote:

> Hi all,
> 
>   can I have reviews for this change that separates `rs_length` and `pending_cards` predictors by young phase?
> 
> Some notes:
> * this does not intend to improve the predictors themselves - i.e. the use of `xxx_diff` and `xxx` predictors and adding them together is kind of weird, imho trying to add a kind of prediction compensation/improvement to act on differences like a (PID-)controller. I also strongly suspect that the `xxx_diff` predictors ought to be allowed to report negative values. This is no attempt to fix this
> * the parameter `for_young_gc` in the analytics API is misnamed. It's not about young gcs, but young_only_phase - I will fix this in a follow-up
> 
> Depends on PR#10649, please review that one first.
> 
> Thanks, 
>   Thomas
> 
> Testing: gha, local testing, tier1-5 with other changes in this patch series, perf testing with other changes in this patch series

`diff/pending_cards/rs_length` are more or less duplicated for young/mixed. I wonder if it's cleaner to encapsulate them in one data structure.

src/hotspot/share/gc/g1/g1Analytics.cpp line 328:

> 326: 
> 327: size_t G1Analytics::predict_pending_cards(bool for_young_gc) const {
> 328:   if (for_young_gc || !enough_samples_available(_young_pending_cards_seq)) {

Why `_young_pending_cards_seq`? I'd expect `_mixed_rs_length_seq` just to match its neighbor.

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

PR: https://git.openjdk.org/jdk/pull/10650



More information about the hotspot-gc-dev mailing list