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

[Man Cao]

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/20181116/8e088e95/attachment.htm>