RFR (M): 8227739: Merge cost predictions for scanning cards and log buffer entries

[Stefan Johansson]

Hi Thomas,

Sorry for taking so long to get to this review.

On 2019-10-22 20:26, Thomas Schatzl wrote:
> Hi all,
> 
>    can I have reviews for this change that aligns the cost predictions 
> to the way we do evacuations, i.e. that we first drop all remembered 
> sets onto the card table, and only a fraction of that will be scanned as 
> introduced by JDK-8213108.
> 
> This code adds all the predictions for ratios etc to align to that code 
> in our prediction model too.
> 
> After this change (and all previous) changes just sent out for review, 
> mostly JDK-8228609 (which is a prerequisite for this change), 
> predictions are a bit (noticably) better than before :)
> 
> CR:
> https://bugs.openjdk.java.net/browse/JDK-8227739
> Webrev:
> http://cr.openjdk.java.net/~tschatzl/8227739/webrev/
Looks good in general, just some small comments:
src/hotspot/share/gc/g1/g1Analytics.cpp
---
255   if (for_young_gc || _mixed_cost_per_card_merge_ms_seq->num() < 3)

We have a few of these "seq->num() < 3" checks, what do you think about 
adding a helper for those? Something like, ready_for_prediction(seq) and 
do:
for_young_gc || !ready_for_prediction(_mixed_cost_per_card_merge_ms_seq)
---

src/hotspot/share/gc/g1/g1Policy.cpp
---
  725     if (total_cards_merged > 10) {
  ...
  738     if (total_cards_scanned > 10) {

Kind of pre-existing, but do you know why we have this limit of 10 in 
these cases. Would be nice to add a comment about it and maybe add a 
constant with some descriptive name.
---

Thanks,
Stefan

> Testing:
> hs-tier1-5, perf testing, pause time keeping improves a little
> 
> Thanks,
>    Thomas