<div dir="ltr"><div dir="ltr">Hi Thomas,<div><br></div>Yes, I totally agree that refactoring code related to UseAdaptiveSizePolicy and UseGCOverheadLimit would be ideal for the long term. They should be independent of each other. I created: <a href="https://bugs.openjdk.java.net/browse/JDK-8212206">https://bugs.openjdk.java.net/browse/JDK-8212206</a></div><div dir="ltr"><br></div><div dir="ltr">I wasn't sure if it is encouraged to do such refactoring to code used by ParallelGC. Good to know that it is encouraged!<br></div><div><br></div><div dir="ltr"><div><div><div dir="ltr" class="gmail_signature"><div dir="ltr">-Man</div></div></div><br></div></div></div><br><div class="gmail_quote"><div dir="ltr">On Mon, Oct 15, 2018 at 2:01 AM Thomas Schatzl <<a href="mailto:thomas.schatzl@oracle.com">thomas.schatzl@oracle.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Man,<br>
<br>
On Fri, 2018-10-12 at 10:47 -0700, Man Cao wrote:<br>
> Sounds good. I'll take a stab at it.<br>
> <br>
> Does initiating the AdaptiveSizePolicy class for G1, or create a<br>
> subclass G1AdaptiveSizePolicy sound like an acceptable high-level<br>
> approach?<br>
> I know it is a non-goal to make UseAdaptiveSizePolicy work for G1,<br>
> but most logic for UseGCOverheadLimit is in the AdaptiveSizePolicy<br>
> class.<br>
> This seems the approach that reuse most existing code and does not<br>
> affect ParallelGC.<br>
> <br>
> A side story is that we have a Google-local patch since JDK6 that<br>
> makes UseGCOverheadLimit work properly for CMS.<br>
> UseAdaptiveSizePolicy doesn't work and is disabled by default for<br>
> CMS, but UseGCOverheadLimit depends on logic in the<br>
> CMSAdaptiveSizePolicy class (which was removed in JDK9) to properly<br>
> collect major GC overhead.<br>
<br>
Your short paragraph simply sound to me that the GC overhead limit<br>
checking is misplaced in the AdaptiveSizePolicy class.<br>
<br>
Since the code is also basically very much geared to a two-generation<br>
heap setup, we should not put it into e.g. CollectorPolicy either.<br>
<br>
What do you think about factoring out this code into a single class to<br>
be reused by the generational collectors? It gets fed using some<br>
sample() methods (for young and full gc each; feel free to adjust the<br>
name) by the collectors, and provides the recent average costs (I saw<br>
that AdaptiveSizePolicy uses them) and that check_gc_overhead_limit()<br>
method?<br>
<br>
Then this functionality could also be used properly by CMS in all of<br>
its modes, i.e. whether using AdaptiveSizePolicy or not I believe.<br>
<br>
Note that above is based on some quick look at the code, there might be<br>
complications as usual :) But importing AdaptiveSizePolicy wholesale<br>
into G1 seems very ugly.<br>
<br>
Thanks,<br>
  Thomas<br>
<br>
<br>
</blockquote></div>