RFR: bug: Timely Reducing Unused Committed Memory

[Thomas Schatzl]

Hi,

On Mon, 2018-08-20 at 13:53 +0200, Rodrigo Bruno wrote:
> Hi,
> 
> please see inline:
> 
> 2018-08-20 12:10 GMT+02:00 Thomas Schatzl <thomas.schatzl at oracle.com>
> :
> > Hi,
> > 
> > On Sun, 2018-08-19 at 22:25 +0200, Rodrigo Bruno wrote:
> > > Hi Thomas,
> > > 
> > > I am just wrapping up the patch with the new changes.
> > > 
> > > Three questions:
> > > 
[...]
> > > 
> > > 2 - what checks should we have inside should_idle_compaction
> > > (previous should_gc)?
> > > Right now we have:
> > > a) time since last compaction attempt, 
> > > b) committed memory above user-defined threshold,
> > > c) over committed mem is above used-defined threshold, and 
> > > d) load average is above user-defined threshold.
> > 
> > I think these are the checks as mentioned in the JEP. Do you see a
> > reason to skip one or the other?
> 
> No, sorry for the question. I will maintain as mentioned in the JEP.

We can still easily change their meaning or add to these or remove some
if we think this makes sense. Nothing set in (quite malleable) stone
yet :)
 
> > > 3 - it can happen now that two Concurrent Cycles are triggered
> > > with no collection in between.
> > 
> > I.e. if the idle timeout is hit multiple times with the option to
> > not do a full compaction? Just to make sure I understand the
> > situation.
> 
> Correct.

Did you ever notice an improvement in heap usage after the first such
attempt?
There should be none except for memory held by j.l.ref.References.

What I want to get at, as a later improvement, we could think of just
skipping those then (as except as mentioned for the j.l.ref.References
issue there would be "no" difference).

> > > This breakes debug build asserts because we never
> > > call  G1ConcurrentMark::post_initial_mark.
> > > We cannot call directly from within CM main loop because we are
> > > outside a safepoint. Any ideas?
> > > Should we setup the next phase (i.e. call post_initial_mark) upon
> > > the Cleanup phase?
> > 
> > This will not work: G1ConcurrentMark::post_initial_mark() needs to
> > be called in a safepoint as the initial setup of marking needs to
> > capture the state of the heap atomically wrt to mutators. Forcing
> > an initial-mark gc (with safepoint) seems to be the way to go here.
> > 
> > Look at G1CollectedHeap::attempt_allocation_humongous() to see how
> > this is done for humongous allocations. Basically a call to
> > G1CollectedHeap::collect() with a new, specific
> > "GCCause::g1_idle_timeout_no_fullgc" (for example) value.
> 
> Okay, will do so.

Thanks,
  Thomas