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

[Roman Kennke]

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.

Incremental:
http://cr.openjdk.java.net/~rkennke/8179268/webrev.05.diff/
<http://cr.openjdk.java.net/%7Erkennke/8179268/webrev.05.diff/>
Full:
http://cr.openjdk.java.net/~rkennke/8179268/webrev.05
<http://cr.openjdk.java.net/%7Erkennke/8179268/webrev.05>

Good now?

Thanks,
Roman

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