RFR: JDK-8151604: Rely on options range checking rather than explict checks

Bengt Rutisson bengt.rutisson at oracle.com
Fri Mar 11 12:38:53 UTC 2016


Hi Derek,

Thanks for looking at this!

On 2016-03-10 17:03, Derek White wrote:
> On 3/10/16 4:59 AM, Bengt Rutisson wrote:
>>
>> Hi everyone,
>>
>> Could I have a couple of reviews for this change?
>>
>> http://cr.openjdk.java.net/~brutisso/8151604/webrev.00/
>> https://bugs.openjdk.java.net/browse/JDK-8151604
>>
>> With the new range checking in the argument parsing we don't need to 
>> have explicit checks in the code to handle invalid arguments.
>>
>> Thanks,
>> Bengt
>
> Great idea. Only comment is that this can go further (I only checked 
> g1CollectorPolicy):
>
> g1CollectorPolicy.cpp:
> - Line 186, can change tests of MaxGCPauseMillis to guarantee(), because:
>           range(1, max_uintx)

Fixed.

> - Line 192, can change tests of GCPauseIntervalMillis to guarantee(), 
> because:
>           OK, this doesn't have a range defined, but it should: 
> range(1, max_uintx)

This code only checks that it is >= 1, which is exactly what 
GCPauseIntervalMillisConstraintFunc does. So, I changed this to a 
guarantee as well.

>
> The checks from lines 220-228 are probably covered by the constraint 
> functions (MaxGCPauseMillisConstraintFunc(), etc), but this RFE is 
> about ranges :-)

I changed those as well, even if they are constraint based rather than 
ranges.

>
>  - Line 293, can change tests of SurvivorRatio to guarantee(), because:
>             range(1, max_uintx-2)

Fixed.

There may be more of those that we can fix, but I think this is a good 
start. I mostly wanted to get rid of the warning() calls.

Updated webrev:
http://cr.openjdk.java.net/~brutisso/8151604/webrev.01/

Diff compared to last version:
http://cr.openjdk.java.net/~brutisso/8151604/webrev.00-01.diff/

Thanks,
Bengt

>
>  - Derek

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/attachments/20160311/55b5f62d/attachment.html>


More information about the hotspot-gc-dev mailing list