<div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr">Hi all,<br><br>Could I have reviews for this refactoring change?<br>Webrev: <a href="https://cr.openjdk.java.net/~manc/8212206/webrev.00/" target="_blank">https://cr.openjdk.java.net/~manc/8212206/webrev.00/</a><br>RFE: <a href="https://bugs.openjdk.java.net/browse/JDK-8212206" target="_blank">https://bugs.openjdk.java.net/browse/JDK-8212206</a><br><br>It basically moves AdaptiveSizePolicy::check_gc_overhead_limit() to its own class so it can be shared with collectors that do not support AdaptiveSizePolicy.<br>Each collector can provide its own functions to check if GC time or space overhead has exceeded threshold. It also removes one unused parameter "young_live".<br>I think this is sufficient to share the common code for implementing UseGCOverheadLimit for G1 (JDK-8212084).<br><br>Initially I tried moving all code for collecting GC time overhead in AdaptiveSizePolicy to a separate class, e.g. <minor|major>_collection_<begin|end>().<br>However, G1 already has its own ways to compute GC cost in G1Analytics. For example, G1HeapSizingPolicy::expansion_amount() already uses G1Analytics::_recent_avg_pause_time_ratio for GC cost. It is undesirable to have two ways to compute GC cost in one collector.<br>In general, different collectors have different ways to compute GC time and space overhead. It could be too restrictive to make an interface to define how to collect GC time and space overhead.<br><br>Thus, this patch only moves the code to check GC overhead in a common place. It could be reused to implement UseGCOverheadLimit for collectors such as ZGC in the future.<br><br>There could be further refactoring to do on top of this patch. I'd like to get some opinions on whether they are desirable to do, and if they could be done as part of this RFE:</div><div dir="ltr">(1) The develop flag AdaptiveSizePolicyGCTimeLimitThreshold could be renamed to GCOverheadLimitThreshold to be more accurate. I'm not sure about the process of renaming a develop flag, though.</div><div dir="ltr">(2) The instance of the new class OverheadChecker could be a member of CollectedHeap, instead of a member of AdaptiveSizePolicy. Note that CollectedHeap::mem_allocate() already has a parameter "bool* gc_overhead_limit_was_exceeded".</div><div dir="ltr">(3) The hsperf counter sun.gc.policy.gcTimeLimitExceeded is currently only available to ParallelGC. It can be moved to GCPolicyCounters so it is available to all collectors. Also gcTimeLimitExceeded is a misnomer, gcOverheadLimitExceeded is more accurate.</div><div dir="ltr"><br></div><div dir="ltr">Thanks,<br></div><div dir="ltr"><div>Man</div></div></div></div></div></div></div></div></div></div>