RFR: 8252294: Remove OopsInGenClosure usage from younger_refs_iterate

[Stefan Karlsson]

On 2020-08-25 11:39, stefan.johansson at oracle.com wrote:
> Hi StefanK,
> 
> Yet another nice cleanup :)
> 
> On 2020-08-25 10:12, Stefan Karlsson wrote:
>> Hi all,
>>
>> Please review this patch to remove the usage of OopsInGenClosure from 
>> younger_refs_iterate and related functions.
>>
>> https://cr.openjdk.java.net/~stefank/8252294/webrev.01/
>> https://bugs.openjdk.java.net/browse/JDK-8252294
> 
> Looks good, just a very minor knit:
> src/hotspot/share/gc/shared/cardTableRS.hpp
> ---
>   165   void non_clean_card_iterate_possibly_parallel(Space* sp, 
> HeapWord* gen_boundary,
>   166                                                 MemRegion mr,
>   167                                                 OopIterateClosure* 
> cl, CardTableRS* ct,
>   168                                                 uint n_threads);
> 
> Organize the parameters so that they are all two per line.
> ---

Thanks for the review. I'll clean that up.

StefanK

> 
> Thanks,
> StefanJ
> 
> 
>>
>> Before the patch we had this call sequence:
>>   GenCollectedHeap::young_process_roots
>>   CardTableRS::younger_refs_iterate
>>   Generation::younger_refs_iterate (CardGeneration as the dynamic type)
>>   CardGeneration::younger_refs_in_space_iterate
>>   CardTableRS::younger_refs_in_space_iterate
>>   ...
>>
>> and we passed the OopsInGenClosure down the entire way.
>>
>> After CMS, when we only have one concrete implementation of 
>> GenCollectedHeap, we can simplify this. I turned the code inside out 
>> and we now have:
>>   SerialHeap::young_process_roots (I moved to SerialHeap)
>>   CardGeneration::younger_refs_iterate
>>   CardTableRS::younger_refs_in_space_iterate
>>
>> - Because of this we don't need the virtual 
>> Generation::younger_refs_iterate, nor the unimplemented version in 
>> DefNewGeneration. It suffices to have a concrete 
>> CardGeneration::younger_refs_iterate.
>>
>> - Inside CardTableRS::younger_refs_in_space_iterate we only used 
>> OopsInGenClosure to extract the gen_boundary() value. (The value set 
>> in OopsInGenClosure::set_generation()). Now that I've straightened out 
>> the code we can now directly extract the "gen boundary" value out of 
>> CardGeneration (old_gen) instead of passing this information down 
>> through the closure.
>>
>> - I moved young_process_roots to SerialHeap, since I need to have 
>> access to CardGeneration and GenCollectedHeap only provides a 
>> Generation* field.
>>
>> - Note: SerialHeap::young_process_roots still use OopsInGenClosure. I 
>> have a follow-up patch that changes that. I'd like to keep this patch 
>> relatively easy.
>>
>> - Note: There's a lot of dead code and potential cleanups that can be 
>> made to CardTableRS and GenCollectedHeap. I don't intend to fix that 
>> with this RFE.
>>
>> Local testing and currently running through tier1-7 on Linux.
>>
>> Thanks,
>> StefanK