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

[Thomas Schatzl]

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.

Please update the copyrights on push.

Thanks,
  Thomas