8166276: Refactor gen_process_roots to allow simpler fix for 8165949

Co-authored-by: Stefan Johansson <stefan.johansson@oracle.com>
Reviewed-by: mgerdin, stefank

hotspot/src/share/vm/gc/cms/concurrentMarkSweepGeneration.cpp

@@ -2340,13 +2340,11 @@ void CMSCollector::verify_after_remark_work_1() {
   {
     StrongRootsScope srs(1);
 
-    gch->gen_process_roots(&srs,
+    gch->old_process_roots(&srs,
-                           GenCollectedHeap::OldGen,
                            true,   // young gen as roots
                            GenCollectedHeap::ScanningOption(roots_scanning_options()),
                            should_unload_classes(),
                            &notOlder,
-                           NULL,
                            NULL);
   }
 
@@ -2414,13 +2412,11 @@ void CMSCollector::verify_after_remark_work_2() {
   {
     StrongRootsScope srs(1);
 
-    gch->gen_process_roots(&srs,
+    gch->old_process_roots(&srs,
-                           GenCollectedHeap::OldGen,
                            true,   // young gen as roots
                            GenCollectedHeap::ScanningOption(roots_scanning_options()),
                            should_unload_classes(),
                            &notOlder,
-                           NULL,
                            &cld_closure);
   }
 
@@ -2903,13 +2899,11 @@ void CMSCollector::checkpointRootsInitialWork() {
 
       StrongRootsScope srs(1);
 
-      gch->gen_process_roots(&srs,
+      gch->old_process_roots(&srs,
-                             GenCollectedHeap::OldGen,
                              true,   // young gen as roots
                              GenCollectedHeap::ScanningOption(roots_scanning_options()),
                              should_unload_classes(),
                              &notOlder,
-                             NULL,
                              &cld_closure);
     }
   }
@@ -4290,13 +4284,11 @@ void CMSParInitialMarkTask::work(uint worker_id) {
 
   CLDToOopClosure cld_closure(&par_mri_cl, true);
 
-  gch->gen_process_roots(_strong_roots_scope,
+  gch->old_process_roots(_strong_roots_scope,
-                         GenCollectedHeap::OldGen,
                          false,     // yg was scanned above
                          GenCollectedHeap::ScanningOption(_collector->CMSCollector::roots_scanning_options()),
                          _collector->should_unload_classes(),
                          &par_mri_cl,
-                         NULL,
                          &cld_closure);
   assert(_collector->should_unload_classes()
          || (_collector->CMSCollector::roots_scanning_options() & GenCollectedHeap::SO_AllCodeCache),
@@ -4421,13 +4413,11 @@ void CMSParRemarkTask::work(uint worker_id) {
   // ---------- remaining roots --------------
   _timer.reset();
   _timer.start();
-  gch->gen_process_roots(_strong_roots_scope,
+  gch->old_process_roots(_strong_roots_scope,
-                         GenCollectedHeap::OldGen,
                          false,     // yg was scanned above
                          GenCollectedHeap::ScanningOption(_collector->CMSCollector::roots_scanning_options()),
                          _collector->should_unload_classes(),
                          &par_mrias_cl,
-                         NULL,
                          NULL);     // The dirty klasses will be handled below
 
   assert(_collector->should_unload_classes()
@@ -4970,13 +4960,11 @@ void CMSCollector::do_remark_non_parallel() {
     gch->rem_set()->prepare_for_younger_refs_iterate(false); // Not parallel.
     StrongRootsScope srs(1);
 
-    gch->gen_process_roots(&srs,
+    gch->old_process_roots(&srs,
-                           GenCollectedHeap::OldGen,
                            true,  // young gen as roots
                            GenCollectedHeap::ScanningOption(roots_scanning_options()),
                            should_unload_classes(),
                            &mrias_cl,
-                           NULL,
                            NULL); // The dirty klasses will be handled below
 
     assert(should_unload_classes()

hotspot/src/share/vm/gc/cms/parNewGeneration.cpp

@@ -605,14 +605,10 @@ void ParNewGenTask::work(uint worker_id) {
                                            false);
 
   par_scan_state.start_strong_roots();
-  gch->gen_process_roots(_strong_roots_scope,
+  gch->young_process_roots(_strong_roots_scope,
-                         GenCollectedHeap::YoungGen,
+                           &par_scan_state.to_space_root_closure(),
-                         true,  // Process younger gens, if any, as strong roots.
+                           &par_scan_state.older_gen_closure(),
-                         GenCollectedHeap::SO_ScavengeCodeCache,
+                           &cld_scan_closure);
-                         GenCollectedHeap::StrongAndWeakRoots,
-                         &par_scan_state.to_space_root_closure(),
-                         &par_scan_state.older_gen_closure(),
-                         &cld_scan_closure);
 
   par_scan_state.end_strong_roots();
 

hotspot/src/share/vm/gc/serial/defNewGeneration.cpp

@@ -648,15 +648,10 @@ void DefNewGeneration::collect(bool   full,
     // See: CardTableModRefBSForCTRS::non_clean_card_iterate_possibly_parallel.
     StrongRootsScope srs(0);
 
-    gch->gen_process_roots(&srs,
+    gch->young_process_roots(&srs,
-                           GenCollectedHeap::YoungGen,
+                             &fsc_with_no_gc_barrier,
-                           true,  // Process younger gens, if any,
+                             &fsc_with_gc_barrier,
-                                  // as strong roots.
+                             &cld_scan_closure);
-                           GenCollectedHeap::SO_ScavengeCodeCache,
-                           GenCollectedHeap::StrongAndWeakRoots,
-                           &fsc_with_no_gc_barrier,
-                           &fsc_with_gc_barrier,
-                           &cld_scan_closure);
   }
 
   // "evacuate followers".

hotspot/src/share/vm/gc/serial/genMarkSweep.cpp

@@ -196,13 +196,11 @@ void GenMarkSweep::mark_sweep_phase1(bool clear_all_softrefs) {
   {
     StrongRootsScope srs(1);
 
-    gch->gen_process_roots(&srs,
+    gch->old_process_roots(&srs,
-                           GenCollectedHeap::OldGen,
                            false, // Younger gens are not roots.
                            GenCollectedHeap::SO_None,
                            ClassUnloading,
                            &follow_root_closure,
-                           &follow_root_closure,
                            &follow_cld_closure);
   }
 
@@ -295,12 +293,10 @@ void GenMarkSweep::mark_sweep_phase3() {
   {
     StrongRootsScope srs(1);
 
-    gch->gen_process_roots(&srs,
+    gch->old_process_roots(&srs,
-                           GenCollectedHeap::OldGen,
                            false, // Younger gens are not roots.
                            GenCollectedHeap::SO_AllCodeCache,
-                           GenCollectedHeap::StrongAndWeakRoots,
+                           false,
-                           &adjust_pointer_closure,
                            &adjust_pointer_closure,
                            &adjust_cld_closure);
   }

hotspot/src/share/vm/gc/shared/genCollectedHeap.cpp

@@ -644,46 +644,47 @@ void GenCollectedHeap::process_roots(StrongRootsScope* scope,
   }
 }
 
-void GenCollectedHeap::gen_process_roots(StrongRootsScope* scope,
+void GenCollectedHeap::young_process_roots(StrongRootsScope* scope,
-                                         GenerationType type,
+                                           OopsInGenClosure* root_closure,
+                                           OopsInGenClosure* old_gen_closure,
+                                           CLDClosure* cld_closure) {
+  MarkingCodeBlobClosure mark_code_closure(root_closure, CodeBlobToOopClosure::FixRelocations);
+
+  process_roots(scope, SO_ScavengeCodeCache, root_closure, root_closure,
+                cld_closure, cld_closure, &mark_code_closure);
+
+  if (!_process_strong_tasks->is_task_claimed(GCH_PS_younger_gens)) {
+    root_closure->reset_generation();
+  }
+
+  // When collection is parallel, all threads get to cooperate to do
+  // old generation scanning.
+  old_gen_closure->set_generation(_old_gen);
+  rem_set()->younger_refs_iterate(_old_gen, old_gen_closure, scope->n_threads());
+  old_gen_closure->reset_generation();
+
+  _process_strong_tasks->all_tasks_completed(scope->n_threads());
+}
+
+void GenCollectedHeap::old_process_roots(StrongRootsScope* scope,
                                          bool young_gen_as_roots,
                                          ScanningOption so,
                                          bool only_strong_roots,
-                                         OopsInGenClosure* not_older_gens,
+                                         OopsInGenClosure* root_closure,
-                                         OopsInGenClosure* older_gens,
                                          CLDClosure* cld_closure) {
-  const bool is_adjust_phase = !only_strong_roots && !young_gen_as_roots;
+  const bool is_moving_collection = !only_strong_roots && !young_gen_as_roots;
 
-  bool is_moving_collection = false;
+  MarkingCodeBlobClosure mark_code_closure(root_closure, is_moving_collection);
-  if (type == YoungGen || is_adjust_phase) {
+  OopsInGenClosure* weak_roots = only_strong_roots ? NULL : root_closure;
-    // young collections are always moving
-    is_moving_collection = true;
-  }
-
-  MarkingCodeBlobClosure mark_code_closure(not_older_gens, is_moving_collection);
-  OopsInGenClosure* weak_roots = only_strong_roots ? NULL : not_older_gens;
   CLDClosure* weak_cld_closure = only_strong_roots ? NULL : cld_closure;
 
-  process_roots(scope, so,
+  process_roots(scope, so, root_closure, weak_roots, cld_closure, weak_cld_closure, &mark_code_closure);
-                not_older_gens, weak_roots,
-                cld_closure, weak_cld_closure,
-                &mark_code_closure);
 
-  if (young_gen_as_roots) {
+  if (young_gen_as_roots &&
-    if (!_process_strong_tasks->is_task_claimed(GCH_PS_younger_gens)) {
+      !_process_strong_tasks->is_task_claimed(GCH_PS_younger_gens)) {
-      if (type == OldGen) {
+    root_closure->set_generation(_young_gen);
-        not_older_gens->set_generation(_young_gen);
+    _young_gen->oop_iterate(root_closure);
-        _young_gen->oop_iterate(not_older_gens);
+    root_closure->reset_generation();
-      }
-      not_older_gens->reset_generation();
-    }
-  }
-  // When collection is parallel, all threads get to cooperate to do
-  // old generation scanning.
-  if (type == YoungGen) {
-    older_gens->set_generation(_old_gen);
-    rem_set()->younger_refs_iterate(_old_gen, older_gens, scope->n_threads());
-    older_gens->reset_generation();
   }
 
   _process_strong_tasks->all_tasks_completed(scope->n_threads());

hotspot/src/share/vm/gc/shared/genCollectedHeap.hpp

@@ -374,16 +374,7 @@ public:
   // asserted to be this type.
   static GenCollectedHeap* heap();
 
-  // Invoke the "do_oop" method of one of the closures "not_older_gens"
+  // The ScanningOption determines which of the roots
-  // or "older_gens" on root locations for the generations depending on
-  // the type.  (The "older_gens" closure is used for scanning references
-  // from older generations; "not_older_gens" is used everywhere else.)
-  // If "younger_gens_as_roots" is false, younger generations are
-  // not scanned as roots; in this case, the caller must be arranging to
-  // scan the younger generations itself.  (For example, a generation might
-  // explicitly mark reachable objects in younger generations, to avoid
-  // excess storage retention.)
-  // The "so" argument determines which of the roots
   // the closure is applied to:
   // "SO_None" does none;
   enum ScanningOption {
@@ -402,16 +393,21 @@ public:
                      CodeBlobToOopClosure* code_roots);
 
  public:
-  static const bool StrongAndWeakRoots = false;
+  void young_process_roots(StrongRootsScope* scope,
-  static const bool StrongRootsOnly    = true;
+                           OopsInGenClosure* root_closure,
+                           OopsInGenClosure* old_gen_closure,
+                           CLDClosure* cld_closure);
 
-  void gen_process_roots(StrongRootsScope* scope,
+  // If "young_gen_as_roots" is false, younger generations are
-                         GenerationType type,
+  // not scanned as roots; in this case, the caller must be arranging to
+  // scan the younger generations itself.  (For example, a generation might
+  // explicitly mark reachable objects in younger generations, to avoid
+  // excess storage retention.)
+  void old_process_roots(StrongRootsScope* scope,
                          bool young_gen_as_roots,
                          ScanningOption so,
                          bool only_strong_roots,
-                         OopsInGenClosure* not_older_gens,
+                         OopsInGenClosure* root_closure,
-                         OopsInGenClosure* older_gens,
                          CLDClosure* cld_closure);
 
   // Apply "root_closure" to all the weak roots of the system.