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

[Bengt Rutisson]

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