RFR: bug: Timely Reducing Unused Committed Memory

[Thomas Schatzl]

Hi,

On Mon, 2018-08-06 at 10:59 +0200, Rodrigo Bruno wrote:
> Hi Thomas,
> 
> thank you for your reply. No problem, w.r.t reponse times. I can also
> take a while to respond...
> 
> Let me just summarize my ideas for the next version of the patch
> taking into consideration your comments.
> This way we will see if we are both on the same page.
> 
> 1 - move the should_gc check from vmthread loop
> into G1ConcurrentMarkThread. I will use sleep_before_next_cycle
> to trigger periodic checks (tune frequency).

Yes. You might want adapt names a bit.

> 
> 2 - inside should_gc:
>   a) move the frequency test to first place
>   b) use Ticks/Tickspan to measure time

There are two reasons should_gc() would return: one the signalling of
that CGC_lock, the other the timeout and request to shrink the heap.

> 3 - when should_gc returns true,
> trigger G1CollectedHeap::resize_after_full_collection
> from a new VM operation

Yes. That should already work.

> 
> 4 - provide a flag UseFullGCForIdleCompaction to allow a full GC
> instead of just G1CollectedHeap::resize_after_full_collection.

Yes.

> 
> 5 - use "Idle Time Compaction" for GC cause

Yes.

> 
> 6 - create one JUnit test.

Please use jtreg, not junit :)

The test should verify that idle timeout starts the expected VM
operation, and the various other settings impact the behavior as
expected.

Thanks,
  Thomas

> 
> This is what I understood from your email. If you agree, I will
> produce a new version of the patch with the proposed changes.
> 
> cheers,
> rodrigo
> 
> 
> 2018-07-26 13:32 GMT+02:00 Thomas Schatzl <thomas.schatzl at oracle.com>
> :
> > Hi,
> > 
> >   at least one sentence needs further clarification.
> > 
> > On Thu, 2018-07-26 at 13:25 +0200, Thomas Schatzl wrote:
> > > Hi Rodrigo,
> > > 
> > >   first, sorry for taking so long to respond in this thread.
> > > 
> > [...]
> > > 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. 
> > > > 
> > > 
> > >   some comments:
> > > 
> > [...]
> > > - 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.
> > 
> >   ... as GC cause...
> > 
> > It is very important to make sure that this action is easily
> > identifiable for users.
> > 
> > Thanks,
> >   Thomas
> > 
> 
>