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

[Man Cao]

Friendly ping. Could anyone give a second "looks good"?

As for the develop flag
AdaptiveSizePolicyGCTimeLimitThreshold/GCOverheadLimitThreshold, I added a
note about it in https://bugs.openjdk.java.net/browse/JDK-8212084.

-Man


On Tue, Jan 15, 2019 at 6:41 PM Man Cao <manc at google.com> wrote:

> Hi,
>
> I rebased the patch to tip and updated year in some headers to 2019,
> without making any real change:
> http://cr.openjdk.java.net/~manc/8212206/webrev.02/
>
>
> I don't foresee that this will be implemented, or even makes sense, for
>> ZGC. As I see it, this is only a thing STW collectors. For that reason,
>> I don't think it belongs in CollectedHeap. Keeping it as a separate
>> utility class for collectors that want to use it sounds better.
>>
> Sounds good to keep this patch in the current state, without further
> changing the CollectedHeap class.
>
> I haven't looked very closely at the patch, but couldn't help to notice
>> that the option is called "GCOverheapLimitThreshold" (and
>> "AdaptiveSizePolicyGCTimeLimitThreshold" before that), which is a
>> tautology and a not very good description of what it is.
>> How about we take the opportunity to clean this up and completely ditch
>> the "gc_overhead_limit_count" thing and get rid of this option? It's a
>> "develop" option, so it's not available to normal users anyway. Has
>> anyone of you ever used this option and actually find it valuable?
>
> I didn't find any users inside Google that require changing this option.
> That said, some users did complain that UseGCOverheadLimit for ParallelGC
> or CMS is too difficult to get
> triggered, because of the requirement for 5 consecutive full GCs, which is
> set by this option.
> I think if it were a normal "product" option, there will definitely be
> users setting it.
> I never understand why it is a "develop" option. I think we could either
> remove it,
> or make it an "experimental" option.
> I'm leaning towards not removing it for now, as I'm not sure if 5 is still
> a reasonable
> default value for UseGCOverheadLimit for G1.
> How about we decide whether to keep or remove this option after
> JDK-8212084 (UseGCOverheadLimit for G1) is fixed?
>
> Also for the hsperfdata counter change, I created
> https://bugs.openjdk.java.net/browse/JDK-8217221. I will draft a CSR for
> it later.
>
> -Man
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20190126/efd0f77c/attachment.htm>