RFR: 8139800: Remove OopsInGenClosure

Stefan Karlsson stefan.karlsson at oracle.com
Tue Aug 25 10:15:31 UTC 2020


Hi all,

Please review this patch to remove OopsInGenClosure.

https://cr.openjdk.java.net/~stefank/8139800/webrev.01/
https://bugs.openjdk.java.net/browse/JDK-8139800

The patch builds upon the cleanup patches that are current out for 
review: JDK-8252245, JDK-8252289, JDK-8252294.

- The preparation patches remove most external usages of 
OopsInGenClosure. However, there are two usages left: 
oop_since_save_marks_iterate and young_process_roots. These usages call 
set_generation before applying the closure, and reset_generation() 
after. I'll explain below why those calls are not needed anymore:

---

After the recent cleanups, FastScanClosure is the only concrete class of 
OopsInGenClosure. FastScanClosure is used to "scan" the young generation 
(DefNew). But, it is actually used in two different situations, and 
require different barrier code depending on where it is used:

1) Scan the old gen for pointers into the young gen, and apply a store 
barrier.
2) Scan non-old gen parts, including oops in CLDs (and associated Metadata)

This can be seen here, where the passed in boolean corresponds to the 
FastScanClosure::_gc_barrier variable:

  573   FastScanClosure fsc_with_no_gc_barrier(this, false);
  574   FastScanClosure fsc_with_gc_barrier(this, true);

This in turn is used here:

   77 template <class T> inline void FastScanClosure::do_oop_work(T* p) {
   ... < Handling the pointer int young gen >
   87       if (is_scanning_a_cld()) {
   88         do_cld_barrier();
   89       } else if (_gc_barrier) {
   90         // Now call parent closure
   91         do_barrier(p);
   92       }

Where line 91 deals with the (1) case above, and 87-88 deals with the 
(2) case.

The do_barrier function called in 91, is provided by OopsInGenClosure. 
The CLD barrier code is provided by OopsInClassLoaderDataOrGenClosure. 
This class inherits from OopsInGenClosure, but doesn't use the 
functionality. It inherits from OopsInGenClosure so that they both can 
be passed through the same code path.

In the patch common code is extracted into a revised FastScanClosure 
super class, that only performs the young gen scanning code, and 
delegates the barrier code to the derived classes.

Two derived classes are created:
- DefNewYoungerGenClosure provides the old-to-young gen write barrier 
and is used for case (1).
- DefNewScanClosure provides the CLD barrier for case (2).

After the separation, it's apparent that DefNewScanClosure doesn't use 
the OopsInGenClosure code, and the only external usages are calls to 
set_generation/reset_generation. There are no actual usages of the 
values set by those functions. The set_generation/reset_generation calls 
that can be removed are here:

SerialHeap::young_process_roots:
...
    if (_process_strong_tasks->try_claim_task(GCH_PS_younger_gens)) {
-    root_closure->reset_generation();
    }

and  template <typename OopClosureType>

  void DefNewGeneration::oop_since_save_marks_iterate(OopClosureType* cl) {
-  cl->set_generation(this);
    eden()->oop_since_save_marks_iterate(cl);
    to()->oop_since_save_marks_iterate(cl);
    from()->oop_since_save_marks_iterate(cl);
-  cl->reset_generation();


DefNewYoungerGenClosure does use the variables changed by 
set_generation/reset_generation, but the only callers always pass in the 
old generation:

  void TenuredGeneration::oop_since_save_marks_iterate(OopClosureType* 
blk) {
-  blk->set_generation(this);
    _the_space->oop_since_save_marks_iterate(blk);
-  blk->reset_generation();


and:

void SerialHeap::young_process_roots
...
-  old_gen_closure->set_generation(_old_gen);
    rem_set()->at_younger_refs_iterate();
    old_gen()->younger_refs_iterate(old_gen_closure, scope->n_threads());
-  old_gen_closure->reset_generation();

So, instead of doing this over and over again, the code is moved to the 
constructor. After that, there's no external usages of OopsInGenClosure, 
and the class can be removed from the class hierarchy.

---

- I've renamed some of the variables to make it more apparent what 
generation the code is dealing with.

- Note: The FastScanClosure and its derived classes should be moved to 
gc/serial, but I've left that for a separate RFE.

- Note: GCH_PS_younger_gens should probably be removed, but I've left 
that cleanup for a separate RFE.

Testing: Testing on tier1-tier7 on Linux with forced usages of 
-XX:+UseSerialGC.

Thanks,
StefanK


More information about the hotspot-gc-dev mailing list