RFR: 8309403: Serial: Remove the useless adaptive size policy in GenCollectedHeap [v2]

[Guoxiong Li]

On Wed, 7 Jun 2023 13:41:04 GMT, Albert Mingkun Yang <ayang at openjdk.org> wrote:

>>> > why do we set it to false again
>>> 
>>> `WB_FullGC` should restore the global env. Otherwise, the following non-whitebox-full-gc cycles can unnecessarily clear soft-refs.
>>>
>>> > If we have a reason to set it to false again, why do other GCs not set it
>>> 
>>> G1 does that inside `WB_FullGC`. ZGC doesn't rely on `SoftRefPolicy::should_clear_all_soft_refs`. Parallel doesn't clear the flag, which I believe is a bug.
>>> 
>>> This inconsistency is probably accidental; keeping the global env intact (essentially what ZGC does) seems the cleanest approach IMO. I wonder if other collectors can also adopt it; in another PR(s) ofc.
>> 
>> Thanks for your answers. I propose the following changes.
>> 
>> In this patch:
>> - Serial GC should use `SoftRefPolicy` instead of `SoftRefGenPolicy`, which is done in this patch now.
>> - Serial GC should restore the global env inside `WB_FullGC`, just like the G1. Because both of them don't implement overheap limit check now.
>> - Don't remove the class `SoftRefGenPolicy` because it will be used by Parallel GC in the future. 
>> 
>> In another patch:
>> - Rename `SoftRefGenPolicy` in order to avoid misunderstanding, such as `GCOverhearLimitSoftRefPolicy`.
>> - Use `SoftRefGenPolicy` (may be renamed at the first step) in Parallel GC.
>> 
>> What do you think?
>
>> Don't remove the class SoftRefGenPolicy because it will be used by Parallel GC in the future.
> 
> I am not sure that using `SoftRefGenPolicy` is the cleanest way to solve the issue in Parallel. Can we check gc-overhead (and other relevant metrics) at gc-pause-start to decide soft-ref-policy, instead of relying on the global bool? (IOW, can GC make the more active decidion, in contrast to letting others tell GC what to do regarding to soft-refs?)
> 
> The approach in `should_clear_soft_references` of `zDriver` seems less intrusive to the global env.

I updated the code just now. I also restore the global env when using Parallel GC, because the Parallel GC doesn't have any implementation like `SoftRefGenPolicy` now (You can think that it solves a bug of Parallel GC).

> Can we check gc-overhead (and other relevant metrics) at gc-pause-start to decide soft-ref-policy, instead of relying on the global bool? (IOW, can GC make the more active decidion, in contrast to letting others tell GC what to do regarding to soft-refs?)
>
> The approach in should_clear_soft_references of zDriver seems less intrusive to the global env.

Seems a good idea. I will file new tickets to follow up. And I think the method `zDriver.cpp::should_clear_soft_references` can be moved into a subclass of `SoftRefPolicy` in order to reuse the code and names, because the `ZCollectedHeap` has the field `SoftRefPolicy  _soft_ref_policy` which is never used. But the detail info should be discussed in its issue.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/14296#discussion_r1221750812