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

Thomas Schatzl thomas.schatzl at oracle.com
Tue May 19 13:02:48 UTC 2020


Hi Kim,

   thanks for your review.

On 18.05.20 16:00, Kim Barrett wrote:
>> On May 18, 2020, at 4:15 AM, Thomas Schatzl <thomas.schatzl at oracle.com> 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
>> Testing:
>> hs-tier1-5
>>
>> Thanks,
>>   Thomas
> 
> Looks good.
> 
> One tiny nit:
> ------------------------------------------------------------------------------
> src/hotspot/share/gc/g1/g1HeapSizingPolicy.cpp
>    74     log_trace(gc, ergo, heap)("Can not expand (heap already fully expanded) "
> 
> [pre-existing]
> s/Can not/Cannot/  -- "Can not" looks weird here.
> 
> ------------------------------------------------------------------------------
> 

Fixed in
http://cr.openjdk.java.net/~tschatzl/8245087/webrev.0_to_1/ (diff)
http://cr.openjdk.java.net/~tschatzl/8245087/webrev.1 (full)

Thanks,
   Thomas


More information about the hotspot-gc-dev mailing list