8261973: Shenandoah: Cleanup/simplify root verifier

Reviewed-by: rkennke

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

@@ -55,69 +55,6 @@ ShenandoahGCStateResetter::~ShenandoahGCStateResetter() {
   _heap->set_concurrent_weak_root_in_progress(_concurrent_weak_root_in_progress);
 }
 
-// Check for overflow of number of root types.
-STATIC_ASSERT((static_cast<uint>(ShenandoahRootVerifier::AllRoots) + 1) > static_cast<uint>(ShenandoahRootVerifier::AllRoots));
-
-ShenandoahRootVerifier::ShenandoahRootVerifier(RootTypes types) : _types(types) {
-  Threads::change_thread_claim_token();
-}
-
-void ShenandoahRootVerifier::excludes(RootTypes types) {
-  _types = static_cast<ShenandoahRootVerifier::RootTypes>(static_cast<uint>(_types) & (~static_cast<uint>(types)));
-}
-
-bool ShenandoahRootVerifier::verify(RootTypes type) const {
-  return (_types & type) == type;
-}
-
-ShenandoahRootVerifier::RootTypes ShenandoahRootVerifier::combine(RootTypes t1, RootTypes t2) {
-  return static_cast<ShenandoahRootVerifier::RootTypes>(static_cast<uint>(t1) | static_cast<uint>(t2));
-}
-
-void ShenandoahRootVerifier::oops_do(OopClosure* oops) {
-  ShenandoahGCStateResetter resetter;
-
-  CodeBlobToOopClosure blobs(oops, !CodeBlobToOopClosure::FixRelocations);
-  if (verify(CodeRoots)) {
-    shenandoah_assert_locked_or_safepoint(CodeCache_lock);
-    CodeCache::blobs_do(&blobs);
-  }
-
-  if (verify(CLDGRoots)) {
-    shenandoah_assert_locked_or_safepoint(ClassLoaderDataGraph_lock);
-    CLDToOopClosure clds(oops, ClassLoaderData::_claim_none);
-    ClassLoaderDataGraph::cld_do(&clds);
-  }
-
-  if (verify(SerialRoots)) {
-    shenandoah_assert_safepoint();
-  }
-
-  if (verify(JNIHandleRoots)) {
-    shenandoah_assert_safepoint();
-    JNIHandles::oops_do(oops);
-    Universe::vm_global()->oops_do(oops);
-  }
-
-  if (verify(WeakRoots)) {
-    shenandoah_assert_safepoint();
-    weak_roots_do(oops);
-  }
-
-  if (ShenandoahStringDedup::is_enabled() && verify(StringDedupRoots)) {
-    shenandoah_assert_safepoint();
-    ShenandoahStringDedup::oops_do_slow(oops);
-  }
-
-  if (verify(ThreadRoots)) {
-    shenandoah_assert_safepoint();
-    // Do thread roots the last. This allows verification code to find
-    // any broken objects from those special roots first, not the accidental
-    // dangling reference from the thread root.
-    Threads::possibly_parallel_oops_do(false, oops, &blobs);
-  }
-}
-
 void ShenandoahRootVerifier::roots_do(OopClosure* oops) {
   ShenandoahGCStateResetter resetter;
   shenandoah_assert_safepoint();
@@ -128,35 +65,37 @@ void ShenandoahRootVerifier::roots_do(OopClosure* oops) {
   CLDToOopClosure clds(oops, ClassLoaderData::_claim_none);
   ClassLoaderDataGraph::cld_do(&clds);
 
-  JNIHandles::oops_do(oops);
+  if (ShenandoahStringDedup::is_enabled()) {
-  Universe::vm_global()->oops_do(oops);
+    ShenandoahStringDedup::oops_do_slow(oops);
+  }
+
+  for (auto id : EnumRange<OopStorageSet::StrongId>()) {
+    OopStorageSet::storage(id)->oops_do(oops);
+  }
 
   // Do thread roots the last. This allows verification code to find
   // any broken objects from those special roots first, not the accidental
   // dangling reference from the thread root.
-  Threads::possibly_parallel_oops_do(true, oops, &blobs);
+  Threads::possibly_parallel_oops_do(true, oops, NULL);
 }
 
 void ShenandoahRootVerifier::strong_roots_do(OopClosure* oops) {
   ShenandoahGCStateResetter resetter;
   shenandoah_assert_safepoint();
 
-  CodeBlobToOopClosure blobs(oops, !CodeBlobToOopClosure::FixRelocations);
-
   CLDToOopClosure clds(oops, ClassLoaderData::_claim_none);
-  ClassLoaderDataGraph::roots_cld_do(&clds, NULL);
+  ClassLoaderDataGraph::always_strong_cld_do(&clds);
 
-  JNIHandles::oops_do(oops);
+  if (ShenandoahStringDedup::is_enabled()) {
-  Universe::vm_global()->oops_do(oops);
+    ShenandoahStringDedup::oops_do_slow(oops);
+  }
 
+  for (auto id : EnumRange<OopStorageSet::StrongId>()) {
+    OopStorageSet::storage(id)->oops_do(oops);
+  }
   // Do thread roots the last. This allows verification code to find
   // any broken objects from those special roots first, not the accidental
   // dangling reference from the thread root.
+  CodeBlobToOopClosure blobs(oops, !CodeBlobToOopClosure::FixRelocations);
   Threads::possibly_parallel_oops_do(true, oops, &blobs);
 }
-
-void ShenandoahRootVerifier::weak_roots_do(OopClosure* cl) {
-  for (auto id : EnumRange<OopStorageSet::WeakId>()) {
-    OopStorageSet::storage(id)->oops_do(cl);
-  }
-}

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

@@ -39,38 +39,11 @@ public:
   ~ShenandoahGCStateResetter();
 };
 
-class ShenandoahRootVerifier : public StackObj {
+class ShenandoahRootVerifier : public AllStatic {
 public:
-  enum RootTypes {
-    None                = 0,
-    SerialRoots         = 1 << 0,
-    ThreadRoots         = 1 << 1,
-    CodeRoots           = 1 << 2,
-    CLDGRoots           = 1 << 3,
-    WeakRoots           = 1 << 4,
-    StringDedupRoots    = 1 << 5,
-    JNIHandleRoots      = 1 << 6,
-    AllRoots            = (SerialRoots | ThreadRoots | CodeRoots | CLDGRoots | WeakRoots | StringDedupRoots | JNIHandleRoots)
-  };
-
-private:
-  RootTypes _types;
-
-public:
-  ShenandoahRootVerifier(RootTypes types = AllRoots);
-
-  void excludes(RootTypes types);
-  void oops_do(OopClosure* cl);
-
   // Used to seed ShenandoahVerifier, do not honor root type filter
-  void roots_do(OopClosure* cl);
+  static void roots_do(OopClosure* cl);
-  void strong_roots_do(OopClosure* cl);
+  static void strong_roots_do(OopClosure* cl);
-
-  static RootTypes combine(RootTypes t1, RootTypes t2);
-private:
-  bool verify(RootTypes type) const;
-
-  void weak_roots_do(OopClosure* cl);
 };
 
 #endif // SHARE_GC_SHENANDOAH_SHENANDOAHROOTVERIFIER_HPP

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

@@ -424,7 +424,6 @@ public:
 class ShenandoahVerifierReachableTask : public AbstractGangTask {
 private:
   const char* _label;
-  ShenandoahRootVerifier* _verifier;
   ShenandoahVerifier::VerifyOptions _options;
   ShenandoahHeap* _heap;
   ShenandoahLivenessData* _ld;
@@ -434,12 +433,10 @@ private:
 public:
   ShenandoahVerifierReachableTask(MarkBitMap* bitmap,
                                   ShenandoahLivenessData* ld,
-                                  ShenandoahRootVerifier* verifier,
                                   const char* label,
                                   ShenandoahVerifier::VerifyOptions options) :
     AbstractGangTask("Shenandoah Verifier Reachable Objects"),
     _label(label),
-    _verifier(verifier),
     _options(options),
     _heap(ShenandoahHeap::heap()),
     _ld(ld),
@@ -464,9 +461,9 @@ public:
                                       ShenandoahMessageBuffer("%s, Roots", _label),
                                       _options);
         if (_heap->unload_classes()) {
-          _verifier->strong_roots_do(&cl);
+          ShenandoahRootVerifier::strong_roots_do(&cl);
         } else {
-          _verifier->roots_do(&cl);
+          ShenandoahRootVerifier::roots_do(&cl);
         }
     }
 
@@ -618,8 +615,7 @@ void ShenandoahVerifier::verify_at_safepoint(const char *label,
                                              VerifyForwarded forwarded, VerifyMarked marked,
                                              VerifyCollectionSet cset,
                                              VerifyLiveness liveness, VerifyRegions regions,
-                                             VerifyGCState gcstate,
+                                             VerifyGCState gcstate) {
-                                             VerifyWeakRoots weak_roots) {
   guarantee(ShenandoahSafepoint::is_at_shenandoah_safepoint(), "only when nothing else happens");
   guarantee(ShenandoahVerify, "only when enabled, and bitmap is initialized in ShenandoahHeap::initialize");
 
@@ -713,8 +709,7 @@ void ShenandoahVerifier::verify_at_safepoint(const char *label,
   // This verifies what application can see, since it only cares about reachable objects.
   size_t count_reachable = 0;
   if (ShenandoahVerifyLevel >= 2) {
-    ShenandoahRootVerifier verifier;
+    ShenandoahVerifierReachableTask task(_verification_bit_map, ld, label, options);
-    ShenandoahVerifierReachableTask task(_verification_bit_map, ld, &verifier, label, options);
     _heap->workers()->run_task(&task);
     count_reachable = task.processed();
   }
@@ -780,8 +775,7 @@ void ShenandoahVerifier::verify_generic(VerifyOption vo) {
           _verify_cset_disable,        // cset may be inconsistent
           _verify_liveness_disable,    // no reliable liveness data
           _verify_regions_disable,     // no reliable region data
-          _verify_gcstate_disable,     // no data about gcstate
+          _verify_gcstate_disable      // no data about gcstate
-          _verify_all_weak_roots
   );
 }
 
@@ -793,8 +787,7 @@ void ShenandoahVerifier::verify_before_concmark() {
           _verify_cset_none,           // UR should have fixed this
           _verify_liveness_disable,    // no reliable liveness data
           _verify_regions_notrash,     // no trash regions
-          _verify_gcstate_stable,      // there are no forwarded objects
+          _verify_gcstate_stable       // there are no forwarded objects
-          _verify_all_weak_roots
   );
 }
 
@@ -806,17 +799,11 @@ void ShenandoahVerifier::verify_after_concmark() {
           _verify_cset_none,           // no references to cset anymore
           _verify_liveness_complete,   // liveness data must be complete here
           _verify_regions_disable,     // trash regions not yet recycled
-          _verify_gcstate_stable,      // mark should have stabilized the heap
+          _verify_gcstate_stable       // mark should have stabilized the heap
-          _verify_all_weak_roots
   );
 }
 
 void ShenandoahVerifier::verify_before_evacuation() {
-  // Concurrent weak roots are evacuated during concurrent phase
-  VerifyWeakRoots verify_weak_roots = _heap->unload_classes() ?
-                                      _verify_serial_weak_roots :
-                                      _verify_all_weak_roots;
-
   verify_at_safepoint(
           "Before Evacuation",
           _verify_forwarded_none,                    // no forwarded references
@@ -824,17 +811,11 @@ void ShenandoahVerifier::verify_before_evacuation() {
           _verify_cset_disable,                      // non-forwarded references to cset expected
           _verify_liveness_complete,                 // liveness data must be complete here
           _verify_regions_disable,                   // trash regions not yet recycled
-          _verify_gcstate_stable,                    // mark should have stabilized the heap
+          _verify_gcstate_stable                     // mark should have stabilized the heap
-          verify_weak_roots
   );
 }
 
 void ShenandoahVerifier::verify_during_evacuation() {
-  // Concurrent weak roots are evacuated during concurrent phase
-  VerifyWeakRoots verify_weak_roots = _heap->unload_classes() ?
-                                      _verify_serial_weak_roots :
-                                      _verify_all_weak_roots;
-
   verify_at_safepoint(
           "During Evacuation",
           _verify_forwarded_allow,    // some forwarded references are allowed
@@ -842,8 +823,7 @@ void ShenandoahVerifier::verify_during_evacuation() {
           _verify_cset_disable,       // some cset references are not forwarded yet
           _verify_liveness_disable,   // liveness data might be already stale after pre-evacs
           _verify_regions_disable,    // trash regions not yet recycled
-          _verify_gcstate_evacuation, // evacuation is in progress
+          _verify_gcstate_evacuation  // evacuation is in progress
-          verify_weak_roots
   );
 }
 
@@ -855,8 +835,7 @@ void ShenandoahVerifier::verify_after_evacuation() {
           _verify_cset_forwarded,      // all cset refs are fully forwarded
           _verify_liveness_disable,    // no reliable liveness data anymore
           _verify_regions_notrash,     // trash regions have been recycled already
-          _verify_gcstate_forwarded,   // evacuation produced some forwarded objects
+          _verify_gcstate_forwarded    // evacuation produced some forwarded objects
-          _verify_all_weak_roots
   );
 }
 
@@ -868,8 +847,7 @@ void ShenandoahVerifier::verify_before_updaterefs() {
           _verify_cset_forwarded,      // all cset refs are fully forwarded
           _verify_liveness_disable,    // no reliable liveness data anymore
           _verify_regions_notrash,     // trash regions have been recycled already
-          _verify_gcstate_forwarded,   // evacuation should have produced some forwarded objects
+          _verify_gcstate_forwarded    // evacuation should have produced some forwarded objects
-          _verify_all_weak_roots
   );
 }
 
@@ -881,8 +859,7 @@ void ShenandoahVerifier::verify_after_updaterefs() {
           _verify_cset_none,           // no cset references, all updated
           _verify_liveness_disable,    // no reliable liveness data anymore
           _verify_regions_nocset,      // no cset regions, trash regions have appeared
-          _verify_gcstate_stable,      // update refs had cleaned up forwarded objects
+          _verify_gcstate_stable       // update refs had cleaned up forwarded objects
-          _verify_all_weak_roots
   );
 }
 
@@ -894,8 +871,7 @@ void ShenandoahVerifier::verify_after_degenerated() {
           _verify_cset_none,           // no cset references
           _verify_liveness_disable,    // no reliable liveness data anymore
           _verify_regions_notrash_nocset, // no trash, no cset
-          _verify_gcstate_stable,       // degenerated refs had cleaned up forwarded objects
+          _verify_gcstate_stable       // degenerated refs had cleaned up forwarded objects
-          _verify_all_weak_roots
   );
 }
 
@@ -907,8 +883,7 @@ void ShenandoahVerifier::verify_before_fullgc() {
           _verify_cset_disable,        // cset might be foobared
           _verify_liveness_disable,    // no reliable liveness data anymore
           _verify_regions_disable,     // no reliable region data here
-          _verify_gcstate_disable,     // no reliable gcstate data
+          _verify_gcstate_disable      // no reliable gcstate data
-          _verify_all_weak_roots
   );
 }
 
@@ -920,8 +895,7 @@ void ShenandoahVerifier::verify_after_fullgc() {
           _verify_cset_none,           // no cset references
           _verify_liveness_disable,    // no reliable liveness data anymore
           _verify_regions_notrash_nocset, // no trash, no cset
-          _verify_gcstate_stable,       // full gc cleaned up everything
+          _verify_gcstate_stable        // full gc cleaned up everything
-          _verify_all_weak_roots
   );
 }
 
@@ -978,33 +952,11 @@ public:
 };
 
 void ShenandoahVerifier::verify_roots_in_to_space() {
-  ShenandoahRootVerifier verifier;
   ShenandoahVerifyInToSpaceClosure cl;
-  verifier.oops_do(&cl);
+  ShenandoahRootVerifier::roots_do(&cl);
-}
-
-void ShenandoahVerifier::verify_roots_in_to_space_except(ShenandoahRootVerifier::RootTypes types) {
-  ShenandoahRootVerifier verifier;
-  verifier.excludes(types);
-  ShenandoahVerifyInToSpaceClosure cl;
-  verifier.oops_do(&cl);
 }
 
 void ShenandoahVerifier::verify_roots_no_forwarded() {
-  ShenandoahRootVerifier verifier;
   ShenandoahVerifyNoForwared cl;
-  verifier.oops_do(&cl);
+  ShenandoahRootVerifier::roots_do(&cl);
-}
-
-void ShenandoahVerifier::verify_roots_no_forwarded(ShenandoahRootVerifier::RootTypes types) {
-  ShenandoahRootVerifier verifier(types);
-  ShenandoahVerifyNoForwared cl;
-  verifier.oops_do(&cl);
-}
-
-void ShenandoahVerifier::verify_roots_no_forwarded_except(ShenandoahRootVerifier::RootTypes types) {
-  ShenandoahRootVerifier verifier;
-  verifier.excludes(types);
-  ShenandoahVerifyNoForwared cl;
-  verifier.oops_do(&cl);
 }

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

@@ -136,12 +136,6 @@ public:
     _verify_gcstate_evacuation
   } VerifyGCState;
 
-  typedef enum {
-    _verify_all_weak_roots,
-    _verify_serial_weak_roots,
-    _verify_concurrent_weak_roots
-  } VerifyWeakRoots;
-
   struct VerifyOptions {
     VerifyForwarded     _verify_forwarded;
     VerifyMarked        _verify_marked;
@@ -149,20 +143,17 @@ public:
     VerifyLiveness      _verify_liveness;
     VerifyRegions       _verify_regions;
     VerifyGCState       _verify_gcstate;
-    VerifyWeakRoots     _verify_weak_roots;
 
     VerifyOptions(VerifyForwarded verify_forwarded,
                   VerifyMarked verify_marked,
                   VerifyCollectionSet verify_collection_set,
                   VerifyLiveness verify_liveness,
                   VerifyRegions verify_regions,
-                  VerifyGCState verify_gcstate,
+                  VerifyGCState verify_gcstate) :
-                  VerifyWeakRoots verify_weak_roots = _verify_all_weak_roots) :
             _verify_forwarded(verify_forwarded), _verify_marked(verify_marked),
             _verify_cset(verify_collection_set),
             _verify_liveness(verify_liveness), _verify_regions(verify_regions),
-            _verify_gcstate(verify_gcstate),
+            _verify_gcstate(verify_gcstate) {}
-            _verify_weak_roots(verify_weak_roots) {}
   };
 
 private:
@@ -172,8 +163,7 @@ private:
                            VerifyCollectionSet cset,
                            VerifyLiveness liveness,
                            VerifyRegions regions,
-                           VerifyGCState gcstate,
+                           VerifyGCState gcstate);
-                           VerifyWeakRoots weakRoots);
 
 public:
   ShenandoahVerifier(ShenandoahHeap* heap, MarkBitMap* verification_bitmap) :
@@ -193,11 +183,8 @@ public:
 
   // Roots should only contain to-space oops
   void verify_roots_in_to_space();
-  void verify_roots_in_to_space_except(ShenandoahRootVerifier::RootTypes types);
 
   void verify_roots_no_forwarded();
-  void verify_roots_no_forwarded(ShenandoahRootVerifier::RootTypes types);
-  void verify_roots_no_forwarded_except(ShenandoahRootVerifier::RootTypes types);
 };
 
 #endif // SHARE_GC_SHENANDOAH_SHENANDOAHVERIFIER_HPP