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

[vladimir kempik]

Hello John.

A customer was running an own java program which keeps allocating memory.

Thanks,
Vladimir
On 21.02.2013 0:13, John Cuthbertson wrote:
> 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
>>>>
>>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20130221/f0a4239a/attachment.htm>