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

[Man Cao]

Hi,

I rebased the patch to latest tip:
https://cr.openjdk.java.net/~manc/8212206/webrev.03/

Could someone give a second "looks good"?

-Man


On Sat, Jan 26, 2019 at 4:35 PM Man Cao <manc at google.com> wrote:

> 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
>>
>