<div dir="ltr"><div dir="ltr"><div dir="ltr">Friendly ping. Could anyone give a second "looks good"?<div><br></div>As for the develop flag AdaptiveSizePolicyGCTimeLimitThreshold/GCOverheadLimitThreshold, I added a note about it in <a href="https://bugs.openjdk.java.net/browse/JDK-8212084">https://bugs.openjdk.java.net/browse/JDK-8212084</a>.<div><div><br clear="all"><div><div dir="ltr" class="gmail_signature"><div dir="ltr">-Man</div></div></div><br></div></div></div></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Jan 15, 2019 at 6:41 PM Man Cao <<a href="mailto:manc@google.com">manc@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div>Hi,</div><div><br></div><div>I rebased the patch to tip and updated year in some headers to 2019, without making any real change:</div><div><a href="http://cr.openjdk.java.net/~manc/8212206/webrev.02/" target="_blank">http://cr.openjdk.java.net/~manc/8212206/webrev.02/</a></div><div><br></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">I don't foresee that this will be implemented, or even makes sense, for <br>ZGC. As I see it, this is only a thing STW collectors. For that reason, <br>I don't think it belongs in CollectedHeap. Keeping it as a separate <br>utility class for collectors that want to use it sounds better.<br></blockquote><div>Sounds good to keep this patch in the current state, without further changing the CollectedHeap class.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">I haven't looked very closely at the patch, but couldn't help to notice <br>that the option is called "GCOverheapLimitThreshold" (and <br>"AdaptiveSizePolicyGCTimeLimitThreshold" before that), which is a <br>tautology and a not very good description of what it is.<br>How about we take the opportunity to clean this up and completely ditch <br>the "gc_overhead_limit_count" thing and get rid of this option? It's a <br>"develop" option, so it's not available to normal users anyway. Has <br>anyone of you ever used this option and actually find it valuable?</blockquote><div>I didn't find any users inside Google that require changing this option.</div>That said, some users did complain that UseGCOverheadLimit for ParallelGC or CMS is too difficult to get<br>triggered, because of the requirement for 5 consecutive full GCs, which is set by this option.</div><div dir="ltr">I think if it were a normal "product" option, there will definitely be users setting it.</div><div dir="ltr">I never understand why it is a "develop" option. I think we could either remove it,</div><div dir="ltr">or make it an "experimental" option.</div><div dir="ltr">I'm leaning towards not removing it for now, as I'm not sure if 5 is still a reasonable<br></div><div>default value for UseGCOverheadLimit for G1.</div><div>How about we decide whether to keep or remove this option after</div><div>JDK-8212084 (UseGCOverheadLimit for G1) is fixed?</div><div><br></div><div dir="ltr"><div>Also for the hsperfdata counter change, I created <a href="https://bugs.openjdk.java.net/browse/JDK-8217221" target="_blank">https://bugs.openjdk.java.net/browse/JDK-8217221</a>. I will draft a CSR for it later.</div><div><br></div><div dir="ltr"><div><div dir="ltr" class="gmail-m_-1398394828689451322gmail-m_3466406192080524826m_-2867842813224295061gmail_signature"><div dir="ltr">-Man</div></div></div></div></div></div></div></div>
</blockquote></div>