[12] RFR (XS): 8215548: G1PeriodicGCSystemLoadThreshold needs to be a double

[Stefan Johansson]

Hi Thomas,

On 2018-12-19 15:58, Thomas Schatzl wrote:
> Hi,
> 
> On Wed, 2018-12-19 at 10:37 +0100, Thomas Schatzl wrote:
>> Hi all,
>>
>>    can I have reviews for this small change that changes the
>> G1PeriodicGCSystemLoadThreshold value type from an uintx to a double?
>>
>> (Manual) Testing by Ruslan from jElastic showed that integers are too
>> coarse for this value, as typical load values you want to compare
>> with are ~0.5.
>>
>> Unfortunately this has not been discovered during review; also
>> testing against a particular load value provided by the OS is hard,
>> so there has not been a regression test for this particular flag
>> where this probably would have been found out. :(
> 
>    there has been a question about the comparison against 0.0 in
> g1YoungRemSetSamplingThread.cpp:75, whether this will catch all cases.
> 
> Any negative values will be blocked already by the "range" specifier,
> but there might be unintentional issues with -0.0. I changed the
> comparison to "> 0.0" to exclude it in any case.
> 
> I updated the webrev in place.
> 
>> CR:
>> https://bugs.openjdk.java.net/browse/JDK-8215548
>> Webrev:
>> http://cr.openjdk.java.net/~tschatzl/8215548/webrev/
Looks good, thanks for fixing this.

Cheers,
Stefan
>> Testing:
>> Manual testing with a hacked gc/g1/TestPeriodicCollection.java test,
>> and the command line range checking tests.
>> hs-tier1-5
> 
> Thanks,
>    Thomas
> 
>