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

Roman Kennke rkennke at redhat.com
Wed Jul 12 13:58:12 UTC 2017

Am 12.07.2017 um 14:20 schrieb Stefan Johansson:
> Hi Roman,
> On 2017-07-12 12:47, Roman Kennke wrote:
>> Am 12.07.2017 um 08:44 schrieb Per Liden:
>>> Hi Roman,
>>> On 2017-07-11 16:17, Stefan Johansson wrote:
>>>> Hi Roman,
>>>> On 2017-07-11 08:34, Per Liden wrote:
>>>>> 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.
>>> I just realized that this doesn't build on linux-i586, which builds a
>>> minimal JVM where INCLUDE_ALL_GCS isn't defined (and will thus not
>>> include parallelScavangeHeap.hpp). Rather than having some ugly #ifdef
>>> INCLUDE_ALL_GCS at the AdaptiveSizePolicyOutput::print() call site I
>>> suggest we keep AdaptiveSizePolicyOutput in adaptiveSizePolicy.hpp for
>>> now.
>> I tried that. Unfortunately, it also requires #ifdef INCLUDE_ALL_GCS to
>> be able to call ParallelScavengeHeap::heap(), or else defeats the
>> purpose of this patch by requiring CollectedHeap to still carry
>> size_policy().. which we don't want. In addition to that, if I try to
>> include parallelScavengeHeap.hpp in adaptiveSizePolicy.hpp, I am getting
>> freaky circular dependency problems. #ifdef INCLUDE_ALL_GCS in java.cpp
>> around AdaptiveSizePolicyOutput seems like the lesser evil... done so
>> here:
>> http://cr.openjdk.java.net/~rkennke/8179268/webrev.04/
>> <http://cr.openjdk.java.net/%7Erkennke/8179268/webrev.04/>
>> Ok?
> I'm no big fan of having #if INCLUDE_ALL_GCS if it can be avoided. A
> few lines below the call to AdaptiveSizePolicyOutput::print(), we call
> Universe::heap()->print_tracing_info(). I think we could move
> AdaptiveSizePolicyOutput::print() into
> ParallelScavengeHeap::print_tracing_info() without running into any
> problems.
> What do you think about that solution?

That's a very good idea!! It alters behaviour slightly (will print
adaptive size policy stuff in hs_err now) but I think that's for the better.


Good now?


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/hotspot-gc-dev/attachments/20170712/0d76508c/attachment.htm>

More information about the hotspot-gc-dev mailing list