<div dir="ltr">Hi Thomas,<div><br></div><div>thank you for your reply. No problem, w.r.t reponse times. I can also take a while to respond.</div><div><br></div><div>Let me just summarize my ideas for the next version of the patch taking into consideration your comments.</div><div>This way we will see if we are both on the same page.</div><div><br></div><div>1 - move the <span style="font-size:12.8px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">should_gc</span> check from vmthread loop into <span style="font-size:12.8px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">G1ConcurrentMarkThread. I will use <span style="text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">sleep_before_next_cycle</span></span></div><div><span style="font-size:12.8px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline"><span style="text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">to trigger periodic checks (tune frequency).</span></span></div><div><span style="font-size:12.8px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline"><br></span></div><div><span style="font-size:12.8px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">2 - inside <span style="text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">should_gc:</span></span></div><div><span style="font-size:12.8px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline"><span style="text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">  a) move the frequency test to first place</span></span></div><div><span style="font-size:12.8px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline"><span style="text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">  b) <span style="background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">use Ticks/Tickspan to measure time</span></span></span></div><div><span style="font-size:12.8px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline"><span style="text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline"><span style="background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline"><br></span></span></span></div><div><span style="font-size:12.8px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline"><span style="text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline"><span style="background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">3 - when should_gc returns true, trigger </span></span></span><span style="font-size:12.8px">G1CollectedHeap::resize_after_</span><wbr style="font-size:12.8px"><span style="font-size:12.8px">full_collection</span></div><div><span style="font-size:12.8px">from a new VM operation</span></div><div><span style="font-size:12.8px"><br></span></div><div><span style="font-size:12.8px">4 - provide a flag <span style="background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">UseFullGCForIdleCompaction to allow a full GC instead of just</span></span></div><div><span style="font-size:12.8px"><span style="background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline"><span style="text-decoration-style:initial;text-decoration-color:initial">G1CollectedHeap::resize_after_</span><wbr style="text-decoration-style:initial;text-decoration-color:initial"><span style="text-decoration-style:initial;text-decoration-color:initial">full_collection.</span><br></span></span></div><div><span style="font-size:12.8px"><span style="background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline"><span style="text-decoration-style:initial;text-decoration-color:initial"><br></span></span></span></div><div><span style="font-size:12.8px"><span style="background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline"><span style="text-decoration-style:initial;text-decoration-color:initial">5 - use "I<span style="background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">dle T</span><span style="background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">ime Compaction" for GC cause</span></span></span></span></div><div><span style="font-size:12.8px"><span style="background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline"><span style="text-decoration-style:initial;text-decoration-color:initial"><span style="background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline"><br></span></span></span></span></div><div><span style="font-size:12.8px"><span style="background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline"><span style="text-decoration-style:initial;text-decoration-color:initial"><span style="background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">6 - create one JUnit test.</span></span></span></span></div><div><span style="font-size:12.8px"><span style="background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline"><span style="text-decoration-style:initial;text-decoration-color:initial"><span style="background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline"><br></span></span></span></span></div><div><span style="font-size:12.8px"><span style="background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline"><span style="text-decoration-style:initial;text-decoration-color:initial"><span style="background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">This is what I understood from your email. If you agree, I will produce a new version of the patch with the proposed changes.</span></span></span></span></div><div><span style="font-size:12.8px"><span style="background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline"><span style="text-decoration-style:initial;text-decoration-color:initial"><span style="background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline"><br></span></span></span></span></div><div><span style="font-size:12.8px"><span style="background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline"><span style="text-decoration-style:initial;text-decoration-color:initial"><span style="background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">cheers,</span></span></span></span></div><div><span style="font-size:12.8px"><span style="background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline"><span style="text-decoration-style:initial;text-decoration-color:initial"><span style="background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">rodrigo</span></span></span></span></div></div><div class="gmail_extra"><br><div class="gmail_quote">2018-07-26 13:32 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>
<br>
  at least one sentence needs further clarification.<br>
<span class=""><br>
On Thu, 2018-07-26 at 13:25 +0200, Thomas Schatzl wrote:<br>
> Hi Rodrigo,<br>
> <br>
>   first, sorry for taking so long to respond in this thread.<br>
> <br>
</span>[...]<br>
<span class="">> On Tue, 2018-06-19 at 20:46 +0200, Rodrigo Bruno wrote:<br>
> > Hi all,<br>
> > <br>
> > here is the first version of our contribution for draft JEP-<br>
> > 8204089. <br>
> > <br>
> <br>
</span>>   some comments:<br>
> <br>
[...]<br>
<span class="">> - looking a bit at other implementations it might be worth to be able<br>
> to customize what is been done when idle is detected.<br>
> <br>
> In most cases it might be sufficient to just shrink the heap (in a<br>
> new VM operation, using the existing code in<br>
> G1CollectedHeap::resize_after_<wbr>full_collection() that already uses<br>
> Min/MaxHeapFreeRatio). A System.gc() seems very intrusive and should<br>
> be optional imho after some thinking; making this optional does not<br>
> seem too much work.<br>
> <br>
> Consider applications with more than a few GB of heap, those will be<br>
> affected a lot (i.e. unresponsive for multiple seconds)<br>
> <br>
> A flag like UseFullGCForIdleCompaction(?) could be used here.<br>
> <br>
> - the change does not make the system.gc() use "Idle" (probably "Idle<br>
> Time Compaction") or similar as suggested in the JEP.<br>
<br>
</span>  ... as GC cause...<br>
<br>
It is very important to make sure that this action is easily<br>
identifiable for users.<br>
<br>
Thanks,<br>
  Thomas<br>
<br>
</blockquote></div><br></div>