RFR: 8156028: G1YoungGenSizer _adaptive_size not correct when setting NewSize and MaxNewSize to the same value

[Stefan Johansson]

On 2016-05-18 20:51, Jon Masamitsu wrote:
> Stefan,
>
> I think your change make the code a little more correct
> but it still confuses me some.  The code that sets _adaptive_size
> to false here
>
>>    30 G1YoungGenSizer::G1YoungGenSizer() : 
>> _sizer_kind(SizerDefaults), _adaptive_size(true),
>>    31         _min_desired_young_length(0), 
>> _max_desired_young_length(0) {
>>    32   if (FLAG_IS_CMDLINE(NewRatio)) {
>>    33     if (FLAG_IS_CMDLINE(NewSize) || FLAG_IS_CMDLINE(MaxNewSize)) {
>>    34       log_warning(gc, ergo)("-XX:NewSize and -XX:MaxNewSize 
>> override -XX:NewRatio");
>>    35     } else {
>>    36       _sizer_kind = SizerNewRatio;
>>    37       _adaptive_size = false;
>>    38       return;
>>    39     }
>>    40   }
>
> says to me that _adaptive_size == true means that G1 is
> being allowed to set the min and max size as it needs to.
> It doesn't say that the min and max are the same (since
> the size is allowed to vary within the bounds set by
> NewRatio and the min and max of the entire heap).
>
> Does that make sense?
>

Not really, _adaptive_size == true means that the young generation size 
can vary between the min and max values specified. This is explained in 
g1YoungGenSizer.hpp. There are two sentences about _adaptive_size in 
there, but they described this as setting a fixed size:
1. There is a special case when NewSize==MaxNewSize. This is interpreted 
as "fixed"
2. If only -XX:NewRatio is set we should use the specified ratio of the 
heap as both min and max. This will be interpreted as "fixed" just like 
the NewSize==MaxNewSize case above.

Point 2 is already correct in the code you pasted above, _adaptive_size 
is set to false when only NewRation is set. Point 1 is what I'm fixing 
in this change.

Do you agree?

Thanks,
Stefan

> Jon
>
>
> On 05/18/2016 07:48 AM, Stefan Johansson wrote:
>>
>> Please review this small change for:
>> https://bugs.openjdk.java.net/browse/JDK-8156028
>>
>> Webrev:
>> http://cr.openjdk.java.net/~sjohanss/8156028/hotspot.00/
>>
>> Summary:
>> The comparison in G1YoungGenSizer is wrong, when NewSize and 
>> MaxNewSize is equal the sizer should not be adaptive.
>>
>> Testing:
>> * JPRT
>> * RBT
>>
>> Thanks,
>> Stefan
>