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

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

>>>
>>> Tested by running hotspot_gc jtreg tests without regressions.
>>>
>>> http://cr.openjdk.java.net/~rkennke/8179268/webrev.00/
>>
>> Please correct me if I'm wrong here but it looks like all the non-G1
>> collectors set the _should_clear_all_soft_refs based on
>> gc_overhead_limit_near.
>> Perhaps the ClearedAllSoftRefs scoped object could be modified to only
>> work with GenCollectorPolicy derived policies (which include parallel
>> *shrugs*) and G1 should just stop worrying about _all_soft_refs_clear.
>> Looking closer, I can't even find G1 code looking at that member so
>> maybe it, too, should be moved to GenCollectorPolicy?
> I can't find any place where should_clear_all_soft_refs() would become
> true for G1.

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.

cheers,
Per

> And, as you mention, G1 doesn't even look at
> all_soft_refs_clear() either. I removed those parts from G1, and moved
> all soft_refs stuff down to GenCollectorPolicy.
>
> I also changed the way the casting accessors as_generation_policy() etc
> work: the as_* accessors now crash with ShouldNotReachHere() when called
> for the wrong policy type, and the is_* accessors now return constant
> true/false based on their type (so that it doesn't crash with
> ShouldNotReachHere() ..). I think this is more useful than the way it's
> been done before.
>
> http://cr.openjdk.java.net/~rkennke/8179268/webrev.01/
> <http://cr.openjdk.java.net/%7Erkennke/8179268/webrev.01/>
>
>
> Tested by: hotspot_gc jtreg tests.
>
> What do you think?
>
> Roman
>