RFR(S): JDK-8028093 - Initial young size is smaller than minimum young size

[Bengt Rutisson]

Jepser,

On 2013-11-11 12:03, Jesper Wilhelmsson wrote:
> Bengt,
>
> The intention of the min_gen1_size argument is to make sure there is 
> space left in the heap for the old generation. The minimum size of the 
> old generation is _min_gen1_size in both cases. In the current code 
> you can see that the argument I want to remove is called min_gen1_size 
> and the code expects to be called with _min_gen1_size at all times. 
> This was also the case before the fix for JDK-8016309, so this bug was 
> actually introduced by the collector policy cleanup.

It does not look like the intent of adjust_gen0_sizes() is to always use 
min_gen1_size. It just has badly named parameters.

To me, adjust_gen0_sizes() looks like it takes three values, gen0_size, 
gen1_size and heap_size, and then adjusts them if they don't match. It's 
odd that it would also adjust for the min_gen1_size in all cases. That 
should be done separately.

Bengt

>
> Thanks,
> /Jesper
>
> Bengt Rutisson skrev 11/11/13 10:32 AM:
>>
>> Hi Jesper,
>>
>> This looks wrong to me.
>>
>> adjust_gen0_sizes() is first called with the min values to adjust 
>> them. Then
>> called with the initial values to adjust them. With your suggested 
>> change we
>> suddenly mix min and initial values when we adjust. That seems very 
>> strange to me.
>>
>> If the issue is that the min and initial values don't match I think 
>> we should
>> add a third step where we explicitly adjust them to match. Something 
>> like:
>>
>> adjust_gen0_sizes(&_min_gen0_size, &_min_gen1_size, _min_heap_byte_size,
>> _min_gen1_size);
>> adjust_gen0_sizes(&_initial_gen0_size, &_initial_gen1_size,
>> _initial_heap_byte_size, _initial_gen1_size);
>> adjust_min_and_initial_values(...);
>>
>> Thanks,
>> Bengt
>>
>> On 2013-11-10 00:17, Jesper Wilhelmsson wrote:
>>> Hi,
>>>
>>> Can I have a couple of reviews of this small bugfix.
>>>
>>> Bug JDK-8028093: https://bugs.openjdk.java.net/browse/JDK-8028093
>>>
>>> The problem is that 
>>> TwoGenerationCollectorPolicy::adjust_gen0_sizes() is
>>> called with the wrong argument in one place. The last argument 
>>> should always
>>> be the value of _min_gen1_size, but in one call it is the 
>>> _initial_gen1_size.
>>>
>>> Since these variables are class local I see no reason to take 
>>> _min_gen1_size
>>> as an argument at all. Apparently it is error prone. The proposed 
>>> fix is
>>> therefore to remove the argument and use _min_gen1_size directly in
>>> adjust_gen0_sizes().
>>>
>>> Webrev: http://cr.openjdk.java.net/~jwilhelm/8028093/webrev/
>>>
>>> Thanks!
>>> /Jesper
>>