Request for review (S): 7113021 G1: automatically enable young gen size auto-tuning when -Xms==-Xmx

[Tony Printezis]

Thanks, ship it.

On 12/21/2011 07:22 AM, Bengt Rutisson wrote:
>
> Tony,
>
> Thanks for the review!
>
> Good comments. I fixed them. Here is an updated webrev:
> http://cr.openjdk.java.net/~brutisso/7113021/webrev.03/
>
> Bengt
>
> On 2011-12-21 10:21, Tony Printezis wrote:
>> Bengt,
>>
>> First, to repeat what I told you privately the other day: I really 
>> liked the fact that you introduced the enum for the sizing policy. It 
>> makes the code much easier to understand. I only have a couple of 
>> minor comments:
>>
>> g1CollectorPolicy.cpp:
>>
>> Instead of having the asserts:
>>
>>  448   assert(G1DefaultMinNewGenPercent<= G1DefaultMaxNewGenPercent, 
>> "Min larger than max");
>>  449   assert(G1DefaultMinNewGenPercent>  0&&  
>> G1DefaultMinNewGenPercent<  100, "Min out of bounds");
>>
>>  456   assert(G1DefaultMinNewGenPercent<= G1DefaultMaxNewGenPercent, 
>> "Min larger than max");
>>  457   assert(G1DefaultMaxNewGenPercent>  0&&  
>> G1DefaultMaxNewGenPercent<  100, "Max out of bounds");
>>
>> at the start of calculate_default_{min,max}_size(), why don't you 
>> just do those checks once in the G1YoungGenSizer constructor (i.e., 
>> treat them like product params)?
>>
>>
>> g1CollectorPolicy.hpp:
>>
>>  100 // If only -XX:NewSize is set we should use the specified value 
>> as the
>>  101 // minimum size for young gen. Still using 50% of the heap as 
>> maximum.
>>  102 //
>>  103 // If only -XX:MaxNewSize is set we should use the specified 
>> value as the
>>  104 // maximum size for young gen. Still using 20% of the heap as 
>> minimum.
>>
>> Instead of saying 20% and 50%, why don't you just refer to the two 
>> develop parameters, as those might change in the future?
>>
>> Apart from that, looks great! Ship it.
>>
>> Tony
>>
>>
>> On 12/19/2011 07:48 AM, Bengt Rutisson wrote:
>>>
>>> Hi everyone,
>>>
>>> Could I have a couple of reviews for this change?
>>> http://cr.openjdk.java.net/~brutisso/7113021/webrev.02/
>>>
>>> The idea is to give G1 some room to allow it to use its heuristics 
>>> for calculating the young gen size even when the heap size is fixed.
>>>
>>> CR:
>>> 7113021 G1: automatically enable young gen size auto-tuning when 
>>> -Xms==-Xmx
>>> http://monaco.us.oracle.com/detail.jsf?cr=7113021
>>> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7113021
>>>
>>> I introduce two new develop flags, G1DefaultMinNewGenPercent and 
>>> G1DefaultMaxNewGenPercent. I will try to get the performance team to 
>>> evaluate some good default values for these flags. Right now they 
>>> are 20 and 50 respectively. This might change if we find that some 
>>> other values are more appropriate.
>>>
>>> Background:
>>>
>>> There are three command line options related to the young gen size: 
>>> NewSize, MaxNewSize and NewRatio (There is also -Xmn, but that is 
>>> just a short form for NewSize==MaxNewSize). G1 will use its internal 
>>> heuristics to calculate the actual young gen size, so these options 
>>> basically only limit the range within which G1 can pick a young gen 
>>> size. Also, these are general options taking byte sizes. G1 will 
>>> internally work with a number of regions instead. So, some rounding 
>>> will occur.
>>>
>>> The idea is that my fix should work pretty much as follows:
>>>
>>> If nothing related to the the young gen size is set on the command 
>>> line we should allow the young gen to be between 20% and 50% of the 
>>> heap size. This means that every time the heap size changes the 
>>> limits for the young gen size will be updated.
>>>
>>> If only -XX:NewSize is set we should use the specified value as the 
>>> minimum size for young gen. Still using 50% of the heap as maximum.
>>>
>>> If only -XX:MaxNewSize is set we should use the specified value as 
>>> the maximum size for young gen. Still using 20% of the heap as minimum.
>>>
>>> If -XX:NewSize and -XX:MaxNewSize are both specified we use these 
>>> values. No updates when the heap size changes. There is a special 
>>> case when NewSize==MaxNewSize. This is interpreted as "fixed" and 
>>> will use a different heuristic for calculating the collection set 
>>> when we do mixed collection.
>>>
>>> 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. But we will update the min 
>>> and max everytime the heap size changes.
>>>
>>> NewSize and MaxNewSize override NewRatio. So, NewRatio is ignored if 
>>> it is combined with either NewSize or MaxNewSize. (A warning message 
>>> is printed.)
>>>
>>> Thanks,
>>> Bengt
>