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

Roman Kennke rkennke at redhat.com
Mon Jul 10 16:35:40 UTC 2017


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/>

Cheers,
Roman

>
> cheers,
> Per
>
>> <http://cr.openjdk.java.net/%7Erkennke/8179268/webrev.00/>
>>
>> Roman
>>




More information about the hotspot-gc-dev mailing list