8245827: Shenandoah: Cleanup Shenandoah code root iterators and root scanner

Reviewed-by: shade

src/hotspot/share/gc/shenandoah/shenandoahCodeRoots.cpp

@@ -423,8 +423,7 @@ ShenandoahCodeRootsIterator::~ShenandoahCodeRootsIterator() {
   }
 }
 
-template<bool CSET_FILTER>
+void ShenandoahCodeRootsIterator::possibly_parallel_blobs_do(CodeBlobClosure *f) {
-void ShenandoahCodeRootsIterator::dispatch_parallel_blobs_do(CodeBlobClosure *f) {
   switch (ShenandoahCodeRootsStyle) {
     case 0: {
       if (_seq_claimed.try_set()) {
@@ -437,7 +436,7 @@ void ShenandoahCodeRootsIterator::dispatch_parallel_blobs_do(CodeBlobClosure *f)
       break;
     }
     case 2: {
-      ShenandoahCodeRootsIterator::fast_parallel_blobs_do<CSET_FILTER>(f);
+      ShenandoahCodeRootsIterator::fast_parallel_blobs_do(f);
       break;
     }
     default:
@@ -445,18 +444,9 @@ void ShenandoahCodeRootsIterator::dispatch_parallel_blobs_do(CodeBlobClosure *f)
   }
 }
 
-void ShenandoahAllCodeRootsIterator::possibly_parallel_blobs_do(CodeBlobClosure *f) {
-  ShenandoahCodeRootsIterator::dispatch_parallel_blobs_do<false>(f);
-}
-
-void ShenandoahCsetCodeRootsIterator::possibly_parallel_blobs_do(CodeBlobClosure *f) {
-  ShenandoahCodeRootsIterator::dispatch_parallel_blobs_do<true>(f);
-}
-
-template <bool CSET_FILTER>
 void ShenandoahCodeRootsIterator::fast_parallel_blobs_do(CodeBlobClosure *f) {
   assert(SafepointSynchronize::is_at_safepoint(), "Must be at safepoint");
   assert(_table_snapshot != NULL, "Sanity");
-  _table_snapshot->parallel_blobs_do<CSET_FILTER>(f);
+  _table_snapshot->parallel_blobs_do(f);
 }
 

src/hotspot/share/gc/shenandoah/shenandoahCodeRoots.hpp

@@ -73,27 +73,12 @@ protected:
   ShenandoahSharedFlag _seq_claimed;
   ShenandoahNMethodTableSnapshot* _table_snapshot;
 
-protected:
+public:
   ShenandoahCodeRootsIterator();
   ~ShenandoahCodeRootsIterator();
 
-  template<bool CSET_FILTER>
-  void dispatch_parallel_blobs_do(CodeBlobClosure *f);
-
-  template<bool CSET_FILTER>
-  void fast_parallel_blobs_do(CodeBlobClosure *f);
-};
-
-class ShenandoahAllCodeRootsIterator : public ShenandoahCodeRootsIterator {
-public:
-  ShenandoahAllCodeRootsIterator() : ShenandoahCodeRootsIterator() {};
   void possibly_parallel_blobs_do(CodeBlobClosure *f);
-};
+  void fast_parallel_blobs_do(CodeBlobClosure *f);
-
-class ShenandoahCsetCodeRootsIterator : public ShenandoahCodeRootsIterator {
-public:
-  ShenandoahCsetCodeRootsIterator() : ShenandoahCodeRootsIterator() {};
-  void possibly_parallel_blobs_do(CodeBlobClosure* f);
 };
 
 class ShenandoahCodeRoots : public AllStatic {

src/hotspot/share/gc/shenandoah/shenandoahConcurrentMark.cpp

@@ -84,9 +84,9 @@ ShenandoahMarkRefsSuperClosure::ShenandoahMarkRefsSuperClosure(ShenandoahObjToSc
 template<UpdateRefsMode UPDATE_REFS>
 class ShenandoahInitMarkRootsTask : public AbstractGangTask {
 private:
-  ShenandoahAllRootScanner* _rp;
+  ShenandoahRootScanner* _rp;
 public:
-  ShenandoahInitMarkRootsTask(ShenandoahAllRootScanner* rp) :
+  ShenandoahInitMarkRootsTask(ShenandoahRootScanner* rp) :
     AbstractGangTask("Shenandoah init mark roots task"),
     _rp(rp) {
   }
@@ -294,7 +294,7 @@ void ShenandoahConcurrentMark::mark_roots(ShenandoahPhaseTimings::Phase root_pha
 
   assert(nworkers <= task_queues()->size(), "Just check");
 
-  ShenandoahAllRootScanner root_proc(nworkers, root_phase);
+  ShenandoahRootScanner root_proc(nworkers, root_phase);
   TASKQUEUE_STATS_ONLY(task_queues()->reset_taskqueue_stats());
   task_queues()->reserve(nworkers);
 

src/hotspot/share/gc/shenandoah/shenandoahNMethod.cpp

@@ -544,6 +544,31 @@ ShenandoahNMethodTableSnapshot::~ShenandoahNMethodTableSnapshot() {
   _list->release();
 }
 
+void ShenandoahNMethodTableSnapshot::parallel_blobs_do(CodeBlobClosure *f) {
+  size_t stride = 256; // educated guess
+
+  ShenandoahNMethod** const list = _list->list();
+
+  size_t max = (size_t)_limit;
+  while (_claimed < max) {
+    size_t cur = Atomic::fetch_and_add(&_claimed, stride);
+    size_t start = cur;
+    size_t end = MIN2(cur + stride, max);
+    if (start >= max) break;
+
+    for (size_t idx = start; idx < end; idx++) {
+      ShenandoahNMethod* nmr = list[idx];
+      assert(nmr != NULL, "Sanity");
+      if (nmr->is_unregistered()) {
+        continue;
+      }
+
+      nmr->assert_alive_and_correct();
+      f->do_code_blob(nmr->nm());
+    }
+  }
+}
+
 void ShenandoahNMethodTableSnapshot::concurrent_nmethods_do(NMethodClosure* cl) {
   size_t stride = 256; // educated guess
 

src/hotspot/share/gc/shenandoah/shenandoahNMethod.hpp

@@ -127,9 +127,7 @@ public:
   ShenandoahNMethodTableSnapshot(ShenandoahNMethodTable* table);
   ~ShenandoahNMethodTableSnapshot();
 
-  template<bool CSET_FILTER>
   void parallel_blobs_do(CodeBlobClosure *f);
-
   void concurrent_nmethods_do(NMethodClosure* cl);
 };
 

src/hotspot/share/gc/shenandoah/shenandoahNMethod.inline.hpp

@@ -124,35 +124,4 @@ ShenandoahNMethod** ShenandoahNMethodList::list() const {
   return _list;
 }
 
-template<bool CSET_FILTER>
-void ShenandoahNMethodTableSnapshot::parallel_blobs_do(CodeBlobClosure *f) {
-  size_t stride = 256; // educated guess
-
-  ShenandoahNMethod** const list = _list->list();
-
-  size_t max = (size_t)_limit;
-  while (_claimed < max) {
-    size_t cur = Atomic::fetch_and_add(&_claimed, stride);
-    size_t start = cur;
-    size_t end = MIN2(cur + stride, max);
-    if (start >= max) break;
-
-    for (size_t idx = start; idx < end; idx++) {
-      ShenandoahNMethod* nmr = list[idx];
-      assert(nmr != NULL, "Sanity");
-      if (nmr->is_unregistered()) {
-        continue;
-      }
-
-      nmr->assert_alive_and_correct();
-
-      if (CSET_FILTER && !nmr->has_cset_oops(_heap)) {
-        continue;
-      }
-
-      f->do_code_blob(nmr->nm());
-    }
-  }
-}
-
 #endif // SHARE_GC_SHENANDOAH_SHENANDOAHNMETHOD_INLINE_HPP

src/hotspot/share/gc/shenandoah/shenandoahRootProcessor.cpp

@@ -175,6 +175,19 @@ void ShenandoahConcurrentStringDedupRoots::oops_do(BoolObjectClosure* is_alive,
   }
 }
 
+ShenandoahCodeCacheRoots::ShenandoahCodeCacheRoots(ShenandoahPhaseTimings::Phase phase) : _phase(phase) {
+  nmethod::oops_do_marking_prologue();
+}
+
+void ShenandoahCodeCacheRoots::code_blobs_do(CodeBlobClosure* blob_cl, uint worker_id) {
+  ShenandoahWorkerTimingsTracker timer(_phase, ShenandoahPhaseTimings::CodeCacheRoots, worker_id);
+  _coderoots_iterator.possibly_parallel_blobs_do(blob_cl);
+}
+
+ShenandoahCodeCacheRoots::~ShenandoahCodeCacheRoots() {
+  nmethod::oops_do_marking_epilogue();
+}
+
 ShenandoahRootProcessor::ShenandoahRootProcessor(ShenandoahPhaseTimings::Phase phase) :
   _heap(ShenandoahHeap::heap()),
   _phase(phase),
@@ -182,6 +195,58 @@ ShenandoahRootProcessor::ShenandoahRootProcessor(ShenandoahPhaseTimings::Phase p
   assert(SafepointSynchronize::is_at_safepoint(), "Must at safepoint");
 }
 
+ShenandoahRootScanner::ShenandoahRootScanner(uint n_workers, ShenandoahPhaseTimings::Phase phase) :
+  ShenandoahRootProcessor(phase),
+  _serial_roots(phase),
+  _thread_roots(phase, n_workers > 1),
+  _code_roots(phase),
+  _vm_roots(phase),
+  _dedup_roots(phase),
+  _cld_roots(phase) {
+}
+
+void ShenandoahRootScanner::roots_do(uint worker_id, OopClosure* oops) {
+  CLDToOopClosure clds_cl(oops, ClassLoaderData::_claim_strong);
+  MarkingCodeBlobClosure blobs_cl(oops, !CodeBlobToOopClosure::FixRelocations);
+  roots_do(worker_id, oops, &clds_cl, &blobs_cl);
+}
+
+void ShenandoahRootScanner::strong_roots_do(uint worker_id, OopClosure* oops) {
+  CLDToOopClosure clds_cl(oops, ClassLoaderData::_claim_strong);
+  MarkingCodeBlobClosure blobs_cl(oops, !CodeBlobToOopClosure::FixRelocations);
+  strong_roots_do(worker_id, oops, &clds_cl, &blobs_cl);
+}
+
+void ShenandoahRootScanner::roots_do(uint worker_id, OopClosure* oops, CLDClosure* clds, CodeBlobClosure* code, ThreadClosure *tc) {
+  assert(!ShenandoahSafepoint::is_at_shenandoah_safepoint() ||
+         !ShenandoahHeap::heap()->unload_classes(),
+          "Expect class unloading when Shenandoah cycle is running");
+  ResourceMark rm;
+
+  _serial_roots.oops_do(oops, worker_id);
+  _vm_roots.oops_do(oops, worker_id);
+
+  assert(clds != NULL, "Only possible with CLD closure");
+  _cld_roots.cld_do(clds, worker_id);
+
+  ShenandoahParallelOopsDoThreadClosure tc_cl(oops, code, tc);
+  _thread_roots.threads_do(&tc_cl, worker_id);
+
+  AlwaysTrueClosure always_true;
+  _dedup_roots.oops_do(&always_true, oops, worker_id);
+}
+
+void ShenandoahRootScanner::strong_roots_do(uint worker_id, OopClosure* oops, CLDClosure* clds, CodeBlobClosure* code, ThreadClosure* tc) {
+  assert(ShenandoahHeap::heap()->unload_classes(), "Should be used during class unloading");
+  ShenandoahParallelOopsDoThreadClosure tc_cl(oops, code, tc);
+  ResourceMark rm;
+
+  _serial_roots.oops_do(oops, worker_id);
+  _vm_roots.oops_do(oops, worker_id);
+  _cld_roots.always_strong_cld_do(clds, worker_id);
+  _thread_roots.threads_do(&tc_cl, worker_id);
+}
+
 ShenandoahRootEvacuator::ShenandoahRootEvacuator(uint n_workers,
                                                  ShenandoahPhaseTimings::Phase phase,
                                                  bool stw_roots_processing,

src/hotspot/share/gc/shenandoah/shenandoahRootProcessor.hpp

@@ -212,11 +212,10 @@ public:
   void oops_do(BoolObjectClosure* is_alive, OopClosure* keep_alive, uint worker_id);
 };
 
-template <typename ITR>
 class ShenandoahCodeCacheRoots {
 private:
   ShenandoahPhaseTimings::Phase _phase;
-  ITR _coderoots_iterator;
+  ShenandoahCodeRootsIterator   _coderoots_iterator;
 public:
   ShenandoahCodeCacheRoots(ShenandoahPhaseTimings::Phase phase);
   ~ShenandoahCodeCacheRoots();
@@ -247,12 +246,11 @@ public:
   ShenandoahHeap* heap() const { return _heap; }
 };
 
-template <typename ITR>
 class ShenandoahRootScanner : public ShenandoahRootProcessor {
 private:
   ShenandoahSerialRoots                                     _serial_roots;
   ShenandoahThreadRoots                                     _thread_roots;
-  ShenandoahCodeCacheRoots<ITR>                             _code_roots;
+  ShenandoahCodeCacheRoots                                  _code_roots;
   ShenandoahVMRoots<false /*concurrent*/ >                  _vm_roots;
   ShenandoahStringDedupRoots                                _dedup_roots;
   ShenandoahClassLoaderDataRoots<false /*concurrent*/, false /*single threaded*/>
@@ -271,9 +269,6 @@ public:
   void roots_do(uint worker_id, OopClosure* oops, CLDClosure* clds, CodeBlobClosure* code, ThreadClosure* tc = NULL);
 };
 
-typedef ShenandoahRootScanner<ShenandoahAllCodeRootsIterator> ShenandoahAllRootScanner;
-typedef ShenandoahRootScanner<ShenandoahCsetCodeRootsIterator> ShenandoahCSetRootScanner;
-
 // This scanner is only for SH::object_iteration() and only supports single-threaded
 // root scanning
 class ShenandoahHeapIterationRootScanner : public ShenandoahRootProcessor {
@@ -286,7 +281,7 @@ private:
   ShenandoahSerialWeakRoots                                _serial_weak_roots;
   ShenandoahWeakRoots<false /*concurrent*/>                _weak_roots;
   ShenandoahConcurrentStringDedupRoots                     _dedup_roots;
-  ShenandoahCodeCacheRoots<ShenandoahAllCodeRootsIterator> _code_roots;
+  ShenandoahCodeCacheRoots                                 _code_roots;
 
 public:
   ShenandoahHeapIterationRootScanner();
@@ -305,7 +300,7 @@ private:
   ShenandoahSerialWeakRoots                                 _serial_weak_roots;
   ShenandoahWeakRoots<false /*concurrent*/>                 _weak_roots;
   ShenandoahStringDedupRoots                                _dedup_roots;
-  ShenandoahCodeCacheRoots<ShenandoahAllCodeRootsIterator>  _code_roots;
+  ShenandoahCodeCacheRoots                                  _code_roots;
   bool                                                      _stw_roots_processing;
   bool                                                      _stw_class_unloading;
 public:
@@ -326,7 +321,7 @@ private:
   ShenandoahSerialWeakRoots                                 _serial_weak_roots;
   ShenandoahWeakRoots<false /*concurrent*/>                 _weak_roots;
   ShenandoahStringDedupRoots                                _dedup_roots;
-  ShenandoahCodeCacheRoots<ShenandoahAllCodeRootsIterator>  _code_roots;
+  ShenandoahCodeCacheRoots                                  _code_roots;
 
 public:
   ShenandoahRootUpdater(uint n_workers, ShenandoahPhaseTimings::Phase phase);
@@ -346,7 +341,7 @@ private:
   ShenandoahSerialWeakRoots                                 _serial_weak_roots;
   ShenandoahWeakRoots<false /*concurrent*/>                 _weak_roots;
   ShenandoahStringDedupRoots                                _dedup_roots;
-  ShenandoahCodeCacheRoots<ShenandoahAllCodeRootsIterator>  _code_roots;
+  ShenandoahCodeCacheRoots                                  _code_roots;
 
 public:
   ShenandoahRootAdjuster(uint n_workers, ShenandoahPhaseTimings::Phase phase);

src/hotspot/share/gc/shenandoah/shenandoahRootProcessor.inline.hpp

@@ -164,22 +164,6 @@ void ShenandoahClassLoaderDataRoots<CONCURRENT, SINGLE_THREADED>::cld_do(CLDClos
   }
 }
 
-template <typename ITR>
-ShenandoahCodeCacheRoots<ITR>::ShenandoahCodeCacheRoots(ShenandoahPhaseTimings::Phase phase) : _phase(phase) {
-  nmethod::oops_do_marking_prologue();
-}
-
-template <typename ITR>
-void ShenandoahCodeCacheRoots<ITR>::code_blobs_do(CodeBlobClosure* blob_cl, uint worker_id) {
-  ShenandoahWorkerTimingsTracker timer(_phase, ShenandoahPhaseTimings::CodeCacheRoots, worker_id);
-  _coderoots_iterator.possibly_parallel_blobs_do(blob_cl);
-}
-
-template <typename ITR>
-ShenandoahCodeCacheRoots<ITR>::~ShenandoahCodeCacheRoots() {
-  nmethod::oops_do_marking_epilogue();
-}
-
 class ShenandoahParallelOopsDoThreadClosure : public ThreadClosure {
 private:
   OopClosure* _f;
@@ -197,63 +181,6 @@ public:
   }
 };
 
-template <typename ITR>
-ShenandoahRootScanner<ITR>::ShenandoahRootScanner(uint n_workers, ShenandoahPhaseTimings::Phase phase) :
-  ShenandoahRootProcessor(phase),
-  _serial_roots(phase),
-  _thread_roots(phase, n_workers > 1),
-  _code_roots(phase),
-  _vm_roots(phase),
-  _dedup_roots(phase),
-  _cld_roots(phase) {
-}
-
-template <typename ITR>
-void ShenandoahRootScanner<ITR>::roots_do(uint worker_id, OopClosure* oops) {
-  CLDToOopClosure clds_cl(oops, ClassLoaderData::_claim_strong);
-  MarkingCodeBlobClosure blobs_cl(oops, !CodeBlobToOopClosure::FixRelocations);
-  roots_do(worker_id, oops, &clds_cl, &blobs_cl);
-}
-
-template <typename ITR>
-void ShenandoahRootScanner<ITR>::strong_roots_do(uint worker_id, OopClosure* oops) {
-  CLDToOopClosure clds_cl(oops, ClassLoaderData::_claim_strong);
-  MarkingCodeBlobClosure blobs_cl(oops, !CodeBlobToOopClosure::FixRelocations);
-  strong_roots_do(worker_id, oops, &clds_cl, &blobs_cl);
-}
-
-template <typename ITR>
-void ShenandoahRootScanner<ITR>::roots_do(uint worker_id, OopClosure* oops, CLDClosure* clds, CodeBlobClosure* code, ThreadClosure *tc) {
-  assert(!ShenandoahSafepoint::is_at_shenandoah_safepoint() ||
-         !ShenandoahHeap::heap()->unload_classes(),
-          "Expect class unloading when Shenandoah cycle is running");
-  ResourceMark rm;
-
-  _serial_roots.oops_do(oops, worker_id);
-  _vm_roots.oops_do(oops, worker_id);
-
-  assert(clds != NULL, "Only possible with CLD closure");
-  _cld_roots.cld_do(clds, worker_id);
-
-  ShenandoahParallelOopsDoThreadClosure tc_cl(oops, code, tc);
-  _thread_roots.threads_do(&tc_cl, worker_id);
-
-  AlwaysTrueClosure always_true;
-  _dedup_roots.oops_do(&always_true, oops, worker_id);
-}
-
-template <typename ITR>
-void ShenandoahRootScanner<ITR>::strong_roots_do(uint worker_id, OopClosure* oops, CLDClosure* clds, CodeBlobClosure* code, ThreadClosure* tc) {
-  assert(ShenandoahHeap::heap()->unload_classes(), "Should be used during class unloading");
-  ShenandoahParallelOopsDoThreadClosure tc_cl(oops, code, tc);
-  ResourceMark rm;
-
-  _serial_roots.oops_do(oops, worker_id);
-  _vm_roots.oops_do(oops, worker_id);
-  _cld_roots.always_strong_cld_do(clds, worker_id);
-  _thread_roots.threads_do(&tc_cl, worker_id);
-}
-
 template <typename IsAlive, typename KeepAlive>
 void ShenandoahRootUpdater::roots_do(uint worker_id, IsAlive* is_alive, KeepAlive* keep_alive) {
   CodeBlobToOopClosure update_blobs(keep_alive, CodeBlobToOopClosure::FixRelocations);