Request for review: 8008546: G1: Wrong G1CONFIDENCEPERCENT results in GUARANTEE(VARIANCE() > -1.0) FAILED
john.cuthbertson at oracle.com
Wed Feb 20 20:13:36 UTC 2013
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.
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:
> I applied what you suggested.
> 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.
>> On 2/20/13 2:31 PM, vladimir kempik wrote:
>>> Hi all,
>>> Could I have a couple of reviews for this change?
>>> 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.
More information about the hotspot-gc-dev