RFR: 8179268: Factor out AdaptiveSizePolicy from top-level interfaces CollectorPolicy and CollectedHeap

[Roman Kennke]

Am 10.07.2017 um 14:23 schrieb Per Liden:
> Hi Roman,
>
> On 2017-07-07 10:53, Roman Kennke wrote:
>> Am 05.07.2017 um 13:12 schrieb Mikael Gerdin:
>>> Hi Roman,
>>>
>>> On 2017-07-04 20:47, Roman Kennke wrote:
>>>> AdaptiveSizePolicy is not used/called from outside the GCs, and not
>>>> all
>>>> GCs need them. It makes sense to remove it from the CollectedHeap and
>>>> CollectorPolicy interfaces and move them down to the actual subclasses
>>>> that used them.
>>>>
>>>> I moved AdaptiveSizePolicyOutput to parallelScavengeHeap.hpp, it's
>>>> only
>>>> used/implemented in the parallel GC. Also, I made this class AllStatic
>>>> (was StackObj)
>
> Thanks for cleaning this up.
>
> May I suggest that the changes related to adaptive size policy is kept
> in one patch and the soft reference clearing stuff in another.

Ok... so we can go back to review the first revision of the patch and
deal with the softrefs stuff in a followup?

http://cr.openjdk.java.net/~rkennke/8179268/webrev.00/
<http://cr.openjdk.java.net/%7Erkennke/8179268/webrev.00/>



>
> For G1 it becomes true when calling WB_FullGC, so your patch changes
> the behavior for G1 here. WB_FullGC is meant to clear soft refs, but I
> looked through the tests and can't find any that currently depend on
> this behavior (but I could have missed it). So, I see two options here:
>
> 1) We change the behavior of WB_FullGC to not guarantee any clearing
> of soft refs, in which case WB_FullGC should never call
> set_should_clear_all_soft_refs() for any GC. Having WB_FullGC clear
> soft refs in GCs but not others seems arbitrary and I can't see the
> value in that.
>
> or
>
> 2) We keep the current behavior of WB_FullGC (i.e. always clear soft
> refs). This of course makes the move of
> set_should_clear_all_soft_refs() to GenCollectorPolicy problematic. We
> could consider changing CollectedHeap::collect() to also take a "bool
> clear_soft_ref", or we could say that it's up to each collector to do
> the right thing when they get called with GCCause::_wb_full_gc.

Ok.
I'd argue it's up to the GC. I am not totally famiiar with the WB stuff,
but I'd expect it to do something similar to what would happen if
applications call the usual API, which is, in this case, System.gc(),
which goes through JVM_GC() which in turn calls heap->collect()
*without* setting the set_should_clear_all_soft_refs(). Right?

In any case, if we don't want this stuff under this enhancement ID, then
we'll discuss it under the followup ID, right?

Roman