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 Fri, 2018-12-07 at 07:04 -0800, sangheon.kim at oracle.com wrote:
> Hi Thomas,
> 
> On 12/6/18 5:54 AM, Thomas Schatzl wrote:
> > Hi all,
> > 
> > On Tue, 2018-12-04 at 10:32 +0100, Stefan Johansson wrote:
> > > On 2018-12-03 15:30, Thomas Schatzl wrote:
> > > > Hi Sangheon,
> > > > 
> > 
> > [...]
> > > > All fixed.
> > > > 
> > > > http://cr.openjdk.java.net/~tschatzl/8212657/webrev.1_to_2/
> > > > (diff)
> > > > http://cr.openjdk.java.net/~tschatzl/8212657/webrev.2/ (full)
> > > 
> > > Still good,
> > > StefanJ
> > 
> >    while doing testing of other changes (where I included this
> > change) I
> > found that very rarely the test fails.
> > 
> > This happens if the test machine is loaded, and additional heap
> > resizing happens due to gctimeratio being too high while we are
> > waiting
> > for a periodic gc.
> > 
> > E.g.
> > 
> > - <create garbage>
> > - some gcs happen
> > - concurrent mark starts
> > - g1 finds out that gc time ratio is too high -> expands the heap
> > by
> > itself
> > - concurrent mark finds that we recently expanded too much, shrinks
> > the
> > heap
> > - periodic gc finds no more opportunity to shrink -> test fails
> > 
> > I trimmed the test to never do any GC (not allocating anything) by
> > setting appropriate startup options but still find opportunity to
> > shrink with periodic gcs.
> > 
> > This makes the test much shorter in code and execution time, and
> > much
> > more robust. :)
> > 
> > http://cr.openjdk.java.net/~tschatzl/8212657/webrev.2_to_3/ (diff)
> > http://cr.openjdk.java.net/~tschatzl/8212657/webrev.3/ (full)
> 
> Looks good!

  thanks for your review - but unfortunately this test fails when it is
run during high-load on the machine. The problem is that it happens
that (periodic) GCs may have occurred before getting the initial heap
size reading, which will shrink the heap.

I had to fix this by a) disallowing running the test when there is
significant delay for getting into the main method, e.g. using -Xcomp,
and b) check that after reading the initial heap sizes there were no
GCs that could have possibly shrinked the heap.

If so, just bail out with some explanation.

http://cr.openjdk.java.net/~tschatzl/8212657/webrev.3_to_4/ (diff)
http://cr.openjdk.java.net/~tschatzl/8212657/webrev.4/ (full)

:(

This passes hs-tier1-5 now, also did some local testing forcing the
issue.

Thanks,
  Thomas