RFR (S): 8019902: G1: Use the average heap size rather than the minimum heap size to calculate the region size

[Stefan Johansson]

Looks good!

/Stefan

On 2013-08-29 14:34, Bengt Rutisson wrote:
>
> Hi Thomas,
>
> Thanks for looking at this!
>
> I agree with you about the log message. Here is an updated webrev 
> without the new logging:
>
> http://cr.openjdk.java.net/~brutisso/8019902/webrev.01/
>
> Thanks,
> Bengt
>
>
> On 8/29/13 2:28 PM, Thomas Schatzl wrote:
>> Hi,
>>
>> On Thu, 2013-08-29 at 11:26 +0200, Bengt Rutisson wrote:
>>> Hi everyone,
>>>
>>> Could I have a couple of reviews of this change:
>>>
>>> http://cr.openjdk.java.net/~brutisso/8019902/webrev.00/
>>>
>>> The fact that G1 by default bases its region size on the minimum heap
>>> size means that out of the box the region size will always be 1M. This
>>> is a problem on large machines with lots of memory. We pick a large 
>>> heap
>>> size but get a very small region size. The small regions are 
>>> inefficient
>>> and cause a lot of memory footprint. Normally we aim to get around 2048
>>> regions, but on a machine with a lot of memory we might pick a default
>>> max heap size of 32G, which means that we will get ~32000 regions. This
>>> can lead to out of memory situations - especially on Solaris x86.
>>>
>>> This patch changes the heuristics for picking the region size to use 
>>> the
>>> average between initial heap size (-Xms) and the maximum heap size
>>> (-Xmx). This means that for large heaps we will pick larger region
>>> sizes. In the 32G example we will now pick a region size of 8m which
>>> means that we will have 4000 regions which is more reasonable.
>>    Looks good.
>>
>> One minor nit: I do not think that the additional log message in
>> heapRegion.cpp adds more information than is already printed by other
>> log messages (PrintGCDetails, PrintFlagsFinal, jcmd, and others).
>>
>> This would also not require the change that initializes the G1Log
>> earlier than before.
>>
>> Thanks,
>>    Thomas
>>
>>
>