RFR: 8204089: Timely Reducing Unused Committed Memory

[Thomas Schatzl]

Hi Rodrigo,

  first, sorry for taking so long to respond in this thread.

Unfortunately the JDK11 release and summer vacations made sure that we 
were a bit busy with other matters.

I think this has now somewhat cleared up, although there is still some
potential for further work for JDK11.

On Tue, 2018-06-19 at 20:46 +0200, Rodrigo Bruno wrote:
> Hi all,
> 
> here is the first version of our contribution for draft JEP-8204089. 
> 
> More details at the CR.
> 
> CR:
> https://bugs.openjdk.java.net/browse/JDK-8204089
> Webrev:
> http://cr.openjdk.java.net/~tschatzl/jelastic/pgc/
> 
> Thanks,
> Rodrigo 

  some comments:

- currently the check for whether we should shrink the heap is attached
to the VMThread-loop as a kind of control thread.
This works, but is a bit ugly I guess :)

One option would be to add a dedicated control thread, but at least all
concurrent collectors already have that (Z the ZDirector and Shenandoah
the ShenandoahControlThread, and G1 its G1ConcurrentMarkThread or
G1YoungRemSetSamplingThread), and it might be better to at most provide
 a method for this functionality instead of going full-on separate.

I.e. my suggestion at the moment, since we are limited to G1 too, is to
 use the G1ConcurrentMarkThread to trigger this.
This would make it a bit hard to support this for parallel and serial
gc, but we limited ourselves to G1 support anyway.

There is a "sleep_before_next_cycle" method which could be somewhat
refactored to timeout "frequently" and trigger the action.

- comments for the should_gc() method:
  - it would imho be best to put the frequency check first because to
me it seems the one that most frequently returns false.
  - please use Ticks/Tickspan to measure time for any new code. I know
that G1 uses mostly doubles to store time, but I am in the process to
replace all of the doubles with those.

- looking a bit at other implementations it might be worth to be able
to customize what is been done when idle is detected.

In most cases it might be sufficient to just shrink the heap (in a new
VM operation, using the existing code in
G1CollectedHeap::resize_after_full_collection() that already uses
Min/MaxHeapFreeRatio). A System.gc() seems very intrusive and should be
optional imho after some thinking; making this optional does not seem
too much work.

Consider applications with more than a few GB of heap, those will be
affected a lot (i.e. unresponsive for multiple seconds)

A flag like UseFullGCForIdleCompaction(?) could be used here.

- the change does not make the system.gc() use "Idle" (probably "Idle
Time Compaction") or similar as suggested in the JEP.

- there needs to be at least one junit test that tests this
functionality given different options.

I also reformatted the JEP text just a bit.

Sorry again for the long delay in answering your request.

Thanks,
  Thomas