RFR (M): 8212657: Implementation of JDK-8204089 Promptly Return Unused Committed Memory from G1 [Was: RFR (M): 8212657: Implementation of JDK-8204089 Timely Reduce Unused Committed Memory]

[Thomas Schatzl]

Hi Sangheon,

On Sun, 2018-12-02 at 14:34 -0800, sangheon.kim at oracle.com wrote:
> Hi Thomas,
> 
> On 11/12/18 1:39 AM, Thomas Schatzl wrote:
> > Hi all,
> > 
> > 
[...]
> > CR:
> > https://bugs.openjdk.java.net/browse/JDK-8212657
> > Webrev:
> > http://cr.openjdk.java.net/~tschatzl/8212657/webrev/index.html
> 
> Looks good.
> I have just minor nits:
> 
> ---------------
>   src/hotspot/share/gc/shared/gcCause.cpp
>   src/hotspot/share/gc/shared/gcCause.hpp
>   src/hotspot/share/gc/shared/vmGCOperations.cpp
> - Copyright update
> 
> ---------------
> test/hotspot/jtreg/gc/g1/ihop/TestIHOPErgo.java
> -        runTest(64, 50, true);
> +        runTest(64, 100, true);
> - Why we need this change? This feature is basically disabled.

The only issue I remember is that with an allocation delay of 50 we run
into quite a lot of evacuation failures in this test. But that is
unrelated to this issue. Removed.

> 
> ---------------
>    73             System.out.println("Skipped. Initial heap size is
> too 
> close to max heap size.");
> - The log message slightly makes me confused. The test is skipped 
> because we don't have enough free/available memory to run the test 
> without triggering GC. 'Initial heap size' was looking as 
> 'InitialHeapSize' at first. :)

All fixed.

http://cr.openjdk.java.net/~tschatzl/8212657/webrev.1_to_2/ (diff)
http://cr.openjdk.java.net/~tschatzl/8212657/webrev.2/ (full)

  thanks for your review.

Thanks,
  Thomas