Request for review: 8008546: G1: Wrong G1CONFIDENCEPERCENT results in GUARANTEE(VARIANCE() > -1.0) FAILED

[John Cuthbertson]

Hi Vladimir,

The change looks good to me. But I have a question: which counter was 
tripping the guarantee? I just ran jvm98 with a confidence percent value 
of 200 and 300 and the guarantee didn't fire for me.

I kind of agree with Bengt that moving this kind of error checking 
earlier would be better. But that can be done as a separate CR.

Thanks,

JohnC

On 2/20/2013 7:27 AM, vladimir kempik wrote:
> Hi Bengt,
>
> Thanks for looking at this!
>
> Here is an updated webrev based on your feedback:
>
> http://cr.openjdk.java.net/~mcherkas/vladimir/8008546/webrev.01/
>
> I applied what you suggested.
>
> Thanks,
> Vladimir.
>
> On 20.02.2013 17:54, Bengt Rutisson wrote:
>>
>> Hi Vladimir,
>>
>> This looks very similar to how we treat G1ReservePercent, so I think 
>> it looks good. An alternative would have been to check this earlier 
>> in the initialization phase and update the flag G1ConfidencePercent 
>> so that PrintFlagsFinal would have printed the actual value. But for 
>> consistency I think it looks good this way.
>>
>> I think you can change G1ConfidencePercent to be an uintx instead of 
>> intx (in g1_globals.hpp). In that case you don't need the second if 
>> statment since it can't be less than 0. It is also more consistent 
>> with G1ReservePercent which is an uintx.
>>
>> Thanks,
>> Bengt
>>
>> On 2/20/13 2:31 PM, vladimir kempik wrote:
>>> Hi all,
>>>
>>> Could I have a couple of reviews for this change?
>>>
>>> http://cr.openjdk.java.net/~mcherkas/vladimir/8008546/webrev.00/
>>>
>>> Input value for G1CONFIDENCEPERCENT wasn't checked before using. 
>>> This results in crash sometimes if -XX:+UseG1GC 
>>> -XX:G1ConfidencePercent=200 flags are used. Now checking the value 
>>> same way as it was done for G1ReservePercent. Increase to 0 if 
>>> negative, decrease to 100 if more than 100.
>>>
>>> Thanks,
>>> Vladimir
>>>
>>
>