RFR: 8179268: Factor out AdaptiveSizePolicy from top-level interfaces CollectorPolicy and CollectedHeap
Per Liden
per.liden at oracle.com
Tue Jul 11 06:34:21 UTC 2017
Hi,
On 2017-07-10 18:35, Roman Kennke wrote:
> Hi Per,
>
> thanks for the review!
>
>>
>>> 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)
>>
>> AdaptiveSizePolicyOutput::print() is actually called from
>> runtime/java.cpp also, so it's used outside of ParallelGC. I'm fine
>> with moving it, but we should have the proper #includes in java.cpp.
>>
>> (Your patch doesn't actually build in its current form. I suspect
>> you're using precompiled headers which have a tendency to hide a lot
>> of errors caused by missing includes)
>>
> I added the include.
>
>>>
>>> Tested by running hotspot_gc jtreg tests without regressions.
>>>
>>> http://cr.openjdk.java.net/~rkennke/8179268/webrev.00/
>>
>> collectorPolicy.hpp:
>> --------------------
>> 258 void cleared_all_soft_refs();
>>
>> Please declare this virtual too (that's the best we can do to signal
>> intent until we have C++11/override)
>>
> Ok.
>
>>
>> collectorPolicy.cpp:
>> --------------------
>> 224 this->CollectorPolicy::cleared_all_soft_refs();
>>
>> Please remove "this->" to match the super-call style used in other
>> places in this file.
>
> ok.
>
>
>>
>> Btw, I can sponsor the patch if you want.
>
> Find the updated webrev here:
>
> http://cr.openjdk.java.net/~rkennke/8179268/webrev.03/
> <http://cr.openjdk.java.net/%7Erkennke/8179268/webrev.03/>
Looks good!
(Awaiting a second review before I can push)
cheers,
Per
>
> Cheers,
> Roman
>
>>
>> cheers,
>> Per
>>
>>> <http://cr.openjdk.java.net/%7Erkennke/8179268/webrev.00/>
>>>
>>> Roman
>>>
>
More information about the hotspot-gc-dev
mailing list