Request for review (S): 7113021 G1: automatically enable young gen size auto-tuning when -Xms==-Xmx
John Cuthbertson
john.cuthbertson at oracle.com
Wed Dec 21 18:04:42 UTC 2011
Hi Bengt,
Looks good except for the following couple of items:
g1_globals.hpp:
294 develop(uintx, G1DefaultMinNewGenPercent,
20, \
295 "Percentage (0-100) of the heap size to use as minunimum
" \
296 "young gen
size.") \
Should be "minimum".
298 develop(uintx, G1DefaultMaxNewGenPercent,
50, \
299 "Percentage (0-100) of the heap size to use as minunimum
" \
300 "young gen size.")
Should be "maximum".
g1CollectorPolicy.[ch]pp:
Can you rename calculate_default_min_size() to be something like
"calculate_default_min_region_num()"? Similarly for
calculate_default_max_size(). They return a number of regions rather
than a size in bytes.
Regards,
JohnC
On 12/21/2011 4: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
>
More information about the hotspot-gc-dev
mailing list