RFR: 8198505: Remove CollectorPolicy and its subclasses

Stefan Karlsson stefan.karlsson at oracle.com
Mon Apr 29 11:27:39 UTC 2019


Hi all,

Per had some comments after looking at this:
  http://cr.openjdk.java.net/~stefank/8198505/webrev.03.delta/
  http://cr.openjdk.java.net/~stefank/8198505/webrev.03/

Thanks,
StefanK

On 2019-04-25 11:06, Stefan Karlsson wrote:
> Hi all,
> 
> I realized that I had turned off the CollectoryPolicy gtest. Fixed it with:
>   http://cr.openjdk.java.net/~stefank/8198505/webrev.02.delta/
>   http://cr.openjdk.java.net/~stefank/8198505/webrev.02/
> 
> I didn't change the name of the test, to make it easier to review. So, 
> either I rename it before pushing, or I remove it if we don't think it's 
> worth keeping.
> 
> Thanks
> StefanK
> 
> On 2019-04-17 14:53, Stefan Karlsson wrote:
>> Hi all,
>>
>> Please review this patch to remove CollectorPolicy and its subclasses.
>>
>> https://cr.openjdk.java.net/~stefank/8198505/webrev.01/
>> https://bugs.openjdk.java.net/browse/JDK-8198505
>>
>>  From the RFE:
>> ==========
>> I'd like to propose that we get rid of the CollectorPolicy classes.
>>
>> The CollectorPolicy classes contains a mix of features to support the 
>> CollectedHeaps. A few of them are:
>>
>> * Have virtual functions that either CMS or Serial can override. This 
>> was important when CMS didn't have it's own CollectedHeap. But CMSHeap 
>> was recently added, so this isn't needed anymore.
>>
>> * Has common allocation code for CMS and Serial. Can be moved to 
>> GenCollectedHeap.
>>
>> * Deal with parts of the heap sizing and corresponding flags. A lot of 
>> the flag handling has been moved to the GCArguments classes. We should 
>> move the CollectorPolicy flag handling functions there as well.
>>
>> * Keeps the state for the SoftRefPolicy. Separate this out to it's own 
>> class.
>>
>> * Has the satisfy_failed_metadata_allocation function. No need to be 
>> located in CollectorPolicy.
>> ==========
>>
>> I started this work last year and have already pushed a number of sub 
>> RFEs:
>>   JDK-8198373: Remove CollectorPolicy::is/as functions
>>   JDK-8198507: Remove CollectorPolicy::create_rem_set
>>   JDK-8198509: Move satisfy_failed_metadata_allocation out from 
>> CollectorPolicy
>>   JDK-8198511: Move allocation functions from GenCollectorPolicy to 
>> GenCollectedHeap
>>   JDK-8198515: Extract SoftReferencePolicy code out of CollectorPolicy
>>   JDK-8198525: Move _size_policy out of GenCollectorPolicy into 
>> GenCollectedHeap
>>   JDK-8198528: Move GenerationSpecs from GenCollectorPolicy to 
>> GenCollectedHeap
>>   JDK-8198530: Move _gc_policy_counters from GenCollectorPolicy to 
>> GenCollectedHeap
>>
>> This patch removes the last bits. It mainly moves flag handling and 
>> sizing information over to the GCArguments. Most of the gc flag 
>> handling has already been moved there, so it makes sense to move the 
>> rest over to that sub-system.
>>
>> For Shenandoah I only moved the flag handling code, but left the 
>> ShenandoahCollectorPolicy class as a Shenandoah internal 
>> implementation detail.
>>
>> Some GCs perform the initialization a little bit differently than 
>> others. For example:
>> - Parallel runs some initialization code twice
>> - Some simply uses the default initialize_heap_flags_and_sizes, while 
>> others override it.
>> - G1 and Shenandoah needs the heap size before alignments can be set up.
>>
>> The last item is a bit awkward because there's a circular dependency 
>> between figuring out the heap size and the heap region size. We have 
>> this initialization sequence:
>>
>> void GCArguments::initialize_before_create_heap() {
>>    initialize_alignments();
>>    initialize_heap_flags_and_sizes();
>>    initialize_size_info();
>> }
>>
>> Today G1 and Shenandoah sets up the heap region sizes in 
>> initialize_alignments, to overcome this circular dependencies. There 
>> might an opportunity for further cleanups in this area.
>>
>> Tested with tier1-3 and tier1-7 on Linux x64.
>>
>> Thanks,
>> StefanK



More information about the hotspot-gc-dev mailing list