RFR (M): 8212206: Refactor AdaptiveSizePolicy to separate out code related to GC overhead

[Man Cao]

I see Thomas added "gc-pending-review" tag on JBS for the RFE.
In the future should I add this tag whenever I send an RFR
to hotspot-gc-dev?

Thanks,
Man


On Thu, Dec 6, 2018 at 9:01 PM Man Cao <manc at google.com> wrote:

> Friendly ping.
>
> -Man
>
>
> On Sat, Nov 17, 2018 at 11:10 AM Man Cao <manc at google.com> wrote:
>
>> Hi all,
>>
>> Could I have reviews for this refactoring change?
>> Webrev: https://cr.openjdk.java.net/~manc/8212206/webrev.00/
>> RFE: https://bugs.openjdk.java.net/browse/JDK-8212206
>>
>> It basically moves AdaptiveSizePolicy::check_gc_overhead_limit() to its
>> own class so it can be shared with collectors that do not support
>> AdaptiveSizePolicy.
>> 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".
>> I think this is sufficient to share the common code for implementing
>> UseGCOverheadLimit for G1 (JDK-8212084).
>>
>> Initially I tried moving all code for collecting GC time overhead in
>> AdaptiveSizePolicy to a separate class, e.g.
>> <minor|major>_collection_<begin|end>().
>> 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.
>> 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.
>>
>> 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.
>>
>> 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:
>> (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.
>> (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".
>> (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.
>>
>> Thanks,
>> Man
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20181206/e816f5b0/attachment.htm>