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

[sangheon]

Hi Bengt,

On 03/11/2016 04:38 AM, Bengt Rutisson wrote:
>
> 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.
>
Looks good to me as well.
Thanks for fixing this.

> 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.
I agree that this is a good start.

FYI, we already have the CR that covers this.
https://bugs.openjdk.java.net/browse/JDK-8133649

Thanks,
Sangheon


>
> 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: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20160311/c60629f0/attachment.htm>