<div dir="ltr">Hi,<div><br></div><div>please see inline:</div><div class="gmail_extra"><br><div class="gmail_quote">2018-08-20 12:10 GMT+02:00 Thomas Schatzl <span dir="ltr"><<a href="mailto:thomas.schatzl@oracle.com" target="_blank">thomas.schatzl@oracle.com</a>></span>:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi,<br>
<span class=""><br>
On Sun, 2018-08-19 at 22:25 +0200, Rodrigo Bruno wrote:<br>
> Hi Thomas,<br>
> <br>
</span><span class="">> I am just wrapping up the patch with the new changes.<br>
> <br>
> Three questions:<br>
> <br>
> 1 - regarding "use Ticks/Tickspan to measure time"<br>
> <br>
> Currently I was using CollectedHeap::last_full_<wbr>collection to get the<br>
> time of the last attemp to compact/shrink. Since we are now giving<br>
> the opportunity to shrink the heap without a full GC, would it make<br>
> sense to have a different counter (last_compaction_attemp or<br>
> something) that would be updated<br>
> inside resize_if_necessary_after_<wbr>full_collection?<br>
<br>
</span>That seems fine.<br></blockquote><div><br></div><div>Ack.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class=""><br>
> <br>
> 2 - what checks should we have inside should_idle_compaction<br>
> (previous should_gc)?<br>
> Right now we have:<br>
</span>> a) time since last compaction attempt, <br>
<span class="">> b) committed memory above user-defined threshold,<br>
> c) over committed mem is above used-defined threshold, and <br>
> d) load average is above user-defined threshold.<br>
<br>
</span>I think these are the checks as mentioned in the JEP. Do you see a<br>
reason to skip one or the other?<br></blockquote><div><br></div><div>No, sorry for the question. I will maintain as mentioned in the JEP.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class=""><br>
> 3 - it can happen now that two Concurrent Cycles are triggered with<br>
> no collection in between.<br>
<br>
</span>I.e. if the idle timeout is hit multiple times with the option to not<br>
do a full compaction? Just to make sure I understand the situation.<br></blockquote><div><br></div><div>Correct.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class=""><br>
> This breakes debug build asserts because we never<br>
> call  G1ConcurrentMark::post_<wbr>initial_mark.<br>
> We cannot call directly from within CM main loop because we are<br>
> outside a safepoint. Any ideas?<br>
> Should we setup the next phase (i.e. call post_initial_mark) upon the<br>
> Cleanup phase?<br>
<br>
</span>This will not work: G1ConcurrentMark::post_<wbr>initial_mark() needs to be<br>
called in a safepoint as the initial setup of marking needs to capture<br>
the state of the heap atomically wrt to mutators. Forcing an initial-<br>
mark gc (with safepoint) seems to be the way to go here.<br>
<br>
Look at G1CollectedHeap::attempt_<wbr>allocation_humongous() to see how this<br>
is done for humongous allocations. Basically a call to<br>
G1CollectedHeap::collect() with a new, specific<br>
"GCCause::g1_idle_timeout_no_<wbr>fullgc" (for example) value.<br></blockquote><div><br></div><div>Okay, will do so.</div><div><br></div><div>Thanks,</div><div>rodrigo</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Thanks,<br>
  Thomas<br>
<br>
</blockquote></div><br></div></div>