RFR (S): 8245087: Use ratios instead of percentages in G1HeapSizingPolicy::expansion_amount

[stefan.johansson at oracle.com]

Hi Thomas,

On 2020-05-18 10:15, Thomas Schatzl wrote:
> Hi all,
> 
>    can I have reviews for this change that changes internal calculations 
> to use ratios (i.e. values between 0-1) instead of percentages (0-100)?
> 
> While in this case use of percent or ratios cancels itself out I prefer 
> to use ratios for calculations and if needed percent for displaying.
> 
> There is just no need to scale the ratios we have with 100, apparently 
> done to make the printing code simpler, but I really prefer it the other 
> way.
> 
> In the log message I also unified the code to print the "%" sign without 
> whitespace before (there has been a mix of that).
> 
> Based on 8245086 also out for review.
> 
> CR:
> https://bugs.openjdk.java.net/browse/JDK-8245087
> Webrev:
> http://cr.openjdk.java.net/~tschatzl/8245087/webrev
I like the way you've split up these cleanup, very nice to review :)

This looks good in general, a few small suggestions:
src/hotspot/share/gc/g1/g1HeapSizingPolicy.cpp
---
53 double G1HeapSizingPolicy::scale_with_heap(double pause_time_ratio) {
...
82   const double pause_time_ratio = 1.0 / (1.0 + GCTimeRatio);
83
84   double threshold = scale_with_heap(pause_time_ratio);

Not sure I have better suggestions, but the naming here was a bit 
misleading at first. I though about changing the function name, but I 
think naming the variable/parameter pause_time_threshold would make it 
clearer. Do you agree?
---

In the scale-function, we could add some debug/trace logging that the 
threshold was scaled.
---

No need for a re-reivew and if you don't agree, leave as is.

Thanks,
Stefan



> Testing:
> hs-tier1-5
> 
> Thanks,
>    Thomas