RFR: 8252294: Remove OopsInGenClosure usage from younger_refs_iterate

stefan.johansson at oracle.com stefan.johansson at oracle.com
Tue Aug 25 09:39:10 UTC 2020


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,
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


More information about the hotspot-gc-dev mailing list