RFR/RFC: 8196071: Change G1 Full GC heap and thread sizing ergonomics

[Stefan Johansson]

On 2018-03-26 10:58, Thomas Schatzl wrote:
> Hi,
>
> On Thu, 2018-03-22 at 16:42 +0100, Stefan Johansson wrote:
>> Hi,
>>
>> Please review or comment on this change to let the G1 Full GC
>> calculate the number of worker threads.
>>
>> Links
>> JBS: https://bugs.openjdk.java.net/browse/JDK-8196071
>> Webrev: http://cr.openjdk.java.net/~sjohanss/8196071/00/
>>
>> Summary
>> After making UseDynamicNumberOfGCThreads true by default, having a
>> sane thread sizing model for the G1 Full GC is important. Before this
>> change the Full GC would use the same number of threads as the
>> previous collection, and if being the first just use 1 thread. The
>> proposed change now have the Full GC consider two metrics to get a
>> good number of worker threads.
>>
>> The first metric is to try to avoid to much wasted space. On average
>> each worker used by the parallel full GC will cause half a region of
>> unused memory. To make sure we don't use too many workers we use
>> G1HeapWastePercent to limit the number of workers.
>>
>> The other metric is the one used in AdaptiveSizePolicy, it looks at
>> the flag HeapSizePerGCWorker and limit the number by this. In most
>> cases this is more restrictive than using G1HeapWastePercent, but for
>> small heaps setting a large G1HeapRegionSize, G1HeapWastePercent will
>> come into play.
>>
>> This change also changes the default value of HeapSizePerGCWorker to
>> 32M instead of 64M, this can be reverted if anyone has a good
>> argument to leave it as is. My testing shows that for DaCapo,
>> SpecJVM98 and GCBasher (gc intensive stress test) with small heaps we
>> get better pause times with the flag set to 32M. There might be other
>> arguments, such as not using to much resources on small systems that
>> might also be important so please let me know if you have reasons why
>> the flag should be left unchanged.
>>
>> Testing
>> Functional testing through Mach5 and manual performance testing with
>> small heaps and different values for HeapSizePerGCThread.
>>
>    looks good, although I would prefer to separate the
> HeapSizePerGCWorker change into a separate CR.
Good point. Created JDK-8200228 for this and just realized I probably 
need a CSR for that as well. Here's a new webrev without the flag-change:
http://cr.openjdk.java.net/~sjohanss/8196071/01/

Thanks,
Stefan

> Please update the copyrights on push.
>
> Thanks,
>    Thomas
>