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

Per Liden per.liden at oracle.com
Mon Jul 10 13:59:31 UTC 2017


Hi,

On 2017-07-10 15:36, Roman Kennke wrote:
> 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?

Sounds good, I'll reply to your first mail separately.

>
> 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'm fine with that, as long as we make sure all GCs actually do the same 
thing so that the meaning of GCCause::_wb_full_gc doesn't differs from 
GC to 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?

The WB interface is for whitebox testing, i.e. an interface for tests 
that need to tell the GC to do something more specific than just 
"System.gc()". For example, "do a young GC" (WB_YoungGC) or "clear all 
soft refs and do a full GC" (WB_FullGC).

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

Sounds good! Thanks!

/Per

>
> Roman
>



More information about the hotspot-gc-dev mailing list