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

Roman Kennke rkennke at redhat.com
Wed Jul 12 10:47:41 UTC 2017


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?

The incremental diff between 03 and 04:

diff --git a/src/share/vm/runtime/java.cpp b/src/share/vm/runtime/java.cpp
--- a/src/share/vm/runtime/java.cpp
+++ b/src/share/vm/runtime/java.cpp
@@ -487,7 +487,10 @@
       ClassLoaderDataGraph::dump_on(log.trace_stream());
     }
   }
+
+#if INCLUDE_ALL_GCS
   AdaptiveSizePolicyOutput::print();
+#endif
 
   if (PrintBytecodeHistogram) {
     BytecodeHistogram::print();

Roman



More information about the hotspot-gc-dev mailing list