8229017: ZGC: Various cleanups of ZVerify
authorpliden
Tue, 06 Aug 2019 15:50:12 +0200
changeset 57658 0022b39ae5ae
parent 57657 22e12dd8f21a
child 57659 d349685a98ae
8229017: ZGC: Various cleanups of ZVerify Reviewed-by: stefank, eosterlund
src/hotspot/share/gc/z/zDriver.cpp
src/hotspot/share/gc/z/zHeapIterator.cpp
src/hotspot/share/gc/z/zMark.cpp
src/hotspot/share/gc/z/zRootsIterator.cpp
src/hotspot/share/gc/z/zRootsIterator.hpp
src/hotspot/share/gc/z/zVerify.cpp
src/hotspot/share/gc/z/zVerify.hpp
--- a/src/hotspot/share/gc/z/zDriver.cpp	Tue Aug 06 15:50:00 2019 +0200
+++ b/src/hotspot/share/gc/z/zDriver.cpp	Tue Aug 06 15:50:12 2019 +0200
@@ -211,6 +211,17 @@
   }
 };
 
+class VM_ZVerify : public VM_Operation {
+public:
+  virtual VMOp_Type type() const {
+    return VMOp_ZVerify;
+  }
+
+  virtual void doit() {
+    ZVerify::after_weak_processing();
+  }
+};
+
 ZDriver::ZDriver() :
     _gc_cycle_port(),
     _gc_locker_port() {
@@ -308,10 +319,9 @@
     // Full verification
     VM_Verify op;
     VMThread::execute(&op);
-
   } else if (ZVerifyRoots || ZVerifyObjects) {
     // Limited verification
-    VM_ZVerifyOperation op;
+    VM_ZVerify op;
     VMThread::execute(&op);
   }
 }
--- a/src/hotspot/share/gc/z/zHeapIterator.cpp	Tue Aug 06 15:50:00 2019 +0200
+++ b/src/hotspot/share/gc/z/zHeapIterator.cpp	Tue Aug 06 15:50:12 2019 +0200
@@ -188,19 +188,13 @@
   obj->oop_iterate(&cl);
 }
 
-class ZHeapIterateConcurrentRootsIterator : public ZConcurrentRootsIterator {
-public:
-  ZHeapIterateConcurrentRootsIterator() :
-      ZConcurrentRootsIterator(ClassLoaderData::_claim_other) {}
-};
-
 template <bool VisitWeaks>
 void ZHeapIterator::objects_do(ObjectClosure* cl) {
   ZStatTimerDisable disable;
 
   // Push roots to visit
-  push_roots<ZRootsIterator,                      false /* Concurrent */, false /* Weak */>();
-  push_roots<ZHeapIterateConcurrentRootsIterator, true  /* Concurrent */, false /* Weak */>();
+  push_roots<ZRootsIterator,                     false /* Concurrent */, false /* Weak */>();
+  push_roots<ZConcurrentRootsIteratorClaimOther, true  /* Concurrent */, false /* Weak */>();
   if (VisitWeaks) {
     push_roots<ZWeakRootsIterator,           false /* Concurrent */, true  /* Weak */>();
     push_roots<ZConcurrentWeakRootsIterator, true  /* Concurrent */, true  /* Weak */>();
--- a/src/hotspot/share/gc/z/zMark.cpp	Tue Aug 06 15:50:00 2019 +0200
+++ b/src/hotspot/share/gc/z/zMark.cpp	Tue Aug 06 15:50:12 2019 +0200
@@ -634,17 +634,16 @@
 class ZMarkConcurrentRootsTask : public ZTask {
 private:
   SuspendibleThreadSetJoiner          _sts_joiner;
-  ZConcurrentRootsIterator            _roots;
+  ZConcurrentRootsIteratorClaimStrong _roots;
   ZMarkConcurrentRootsIteratorClosure _cl;
 
 public:
   ZMarkConcurrentRootsTask(ZMark* mark) :
       ZTask("ZMarkConcurrentRootsTask"),
-      _sts_joiner(true /* active */),
-      _roots(ClassLoaderData::_claim_strong),
+      _sts_joiner(),
+      _roots(),
       _cl() {
     ClassLoaderDataGraph_lock->lock();
-    ClassLoaderDataGraph::clear_claimed_marks();
   }
 
   ~ZMarkConcurrentRootsTask() {
--- a/src/hotspot/share/gc/z/zRootsIterator.cpp	Tue Aug 06 15:50:00 2019 +0200
+++ b/src/hotspot/share/gc/z/zRootsIterator.cpp	Tue Aug 06 15:50:12 2019 +0200
@@ -272,6 +272,7 @@
     _vm_handles(this),
     _class_loader_data_graph(this) {
   ZStatTimer timer(ZSubPhaseConcurrentRootsSetup);
+  ClassLoaderDataGraph::clear_claimed_marks(cld_claim);
 }
 
 ZConcurrentRootsIterator::~ZConcurrentRootsIterator() {
--- a/src/hotspot/share/gc/z/zRootsIterator.hpp	Tue Aug 06 15:50:00 2019 +0200
+++ b/src/hotspot/share/gc/z/zRootsIterator.hpp	Tue Aug 06 15:50:12 2019 +0200
@@ -111,9 +111,9 @@
 
 class ZConcurrentRootsIterator {
 private:
-  ZOopStorageIterator        _jni_handles_iter;
-  ZOopStorageIterator        _vm_handles_iter;
-  int                        _cld_claim;
+  ZOopStorageIterator _jni_handles_iter;
+  ZOopStorageIterator _vm_handles_iter;
+  const int           _cld_claim;
 
   void do_jni_handles(ZRootsIteratorClosure* cl);
   void do_vm_handles(ZRootsIteratorClosure* cl);
@@ -130,13 +130,31 @@
   void oops_do(ZRootsIteratorClosure* cl);
 };
 
+class ZConcurrentRootsIteratorClaimStrong : public ZConcurrentRootsIterator {
+public:
+  ZConcurrentRootsIteratorClaimStrong() :
+      ZConcurrentRootsIterator(ClassLoaderData::_claim_strong) {}
+};
+
+class ZConcurrentRootsIteratorClaimOther : public ZConcurrentRootsIterator {
+public:
+  ZConcurrentRootsIteratorClaimOther() :
+      ZConcurrentRootsIterator(ClassLoaderData::_claim_other) {}
+};
+
+class ZConcurrentRootsIteratorClaimNone : public ZConcurrentRootsIterator {
+public:
+  ZConcurrentRootsIteratorClaimNone() :
+      ZConcurrentRootsIterator(ClassLoaderData::_claim_none) {}
+};
+
 class ZWeakRootsIterator {
 private:
   void do_jvmti_weak_export(BoolObjectClosure* is_alive, ZRootsIteratorClosure* cl);
   void do_jfr_weak(BoolObjectClosure* is_alive, ZRootsIteratorClosure* cl);
 
-  ZSerialWeakOopsDo<ZWeakRootsIterator, &ZWeakRootsIterator::do_jvmti_weak_export>  _jvmti_weak_export;
-  ZSerialWeakOopsDo<ZWeakRootsIterator, &ZWeakRootsIterator::do_jfr_weak>           _jfr_weak;
+  ZSerialWeakOopsDo<ZWeakRootsIterator, &ZWeakRootsIterator::do_jvmti_weak_export> _jvmti_weak_export;
+  ZSerialWeakOopsDo<ZWeakRootsIterator, &ZWeakRootsIterator::do_jfr_weak>          _jfr_weak;
 
 public:
   ZWeakRootsIterator();
--- a/src/hotspot/share/gc/z/zVerify.cpp	Tue Aug 06 15:50:00 2019 +0200
+++ b/src/hotspot/share/gc/z/zVerify.cpp	Tue Aug 06 15:50:12 2019 +0200
@@ -23,7 +23,6 @@
 
 #include "precompiled.hpp"
 #include "classfile/classLoaderData.hpp"
-#include "classfile/classLoaderDataGraph.hpp"
 #include "gc/z/zAddress.hpp"
 #include "gc/z/zHeap.inline.hpp"
 #include "gc/z/zOop.hpp"
@@ -31,41 +30,65 @@
 #include "gc/z/zRootsIterator.hpp"
 #include "gc/z/zStat.hpp"
 #include "gc/z/zVerify.hpp"
-#include "memory/allocation.hpp"
 #include "memory/iterator.inline.hpp"
-#include "oops/oop.inline.hpp"
+#include "oops/oop.hpp"
+
+#define BAD_OOP_ARG(o, p)   "Bad oop " PTR_FORMAT " found at " PTR_FORMAT, p2i(o), p2i(p)
 
-#define BAD_OOP_REPORT(addr)                                                \
-    "Bad oop " PTR_FORMAT " found at " PTR_FORMAT ", expected " PTR_FORMAT, \
-    addr, p2i(p), ZAddress::good(addr)
+static void verify_oop(oop* p) {
+  const oop o = RawAccess<>::oop_load(p);
+  if (o != NULL) {
+    const uintptr_t addr = ZOop::to_address(o);
+    guarantee(ZAddress::is_good(addr), BAD_OOP_ARG(o, p));
+    guarantee(oopDesc::is_oop(ZOop::from_address(addr)), BAD_OOP_ARG(o, p));
+  }
+}
 
-class ZVerifyRootsClosure : public ZRootsIteratorClosure {
+static void verify_possibly_weak_oop(oop* p) {
+  const oop o = RawAccess<>::oop_load(p);
+  if (o != NULL) {
+    const uintptr_t addr = ZOop::to_address(o);
+    guarantee(ZAddress::is_good(addr) || ZAddress::is_finalizable_good(addr), BAD_OOP_ARG(o, p));
+    guarantee(oopDesc::is_oop(ZOop::from_address(ZAddress::good(addr))), BAD_OOP_ARG(o, p));
+  }
+}
+
+class ZVerifyRootClosure : public ZRootsIteratorClosure {
 public:
   virtual void do_oop(oop* p) {
-    uintptr_t value = ZOop::to_address(*p);
-
-    if (value == 0) {
-      return;
-    }
+    verify_oop(p);
+  }
 
-    guarantee(!ZAddress::is_finalizable(value), BAD_OOP_REPORT(value));
-    guarantee(ZAddress::is_good(value), BAD_OOP_REPORT(value));
-    guarantee(oopDesc::is_oop(ZOop::from_address(value)), BAD_OOP_REPORT(value));
+  virtual void do_oop(narrowOop*) {
+    ShouldNotReachHere();
   }
-  virtual void do_oop(narrowOop*) { ShouldNotReachHere(); }
 };
 
-template <bool VisitReferents>
 class ZVerifyOopClosure : public ClaimMetadataVisitingOopIterateClosure, public ZRootsIteratorClosure  {
+private:
+  const bool _verify_weaks;
+
 public:
-  ZVerifyOopClosure() :
-      ClaimMetadataVisitingOopIterateClosure(ClassLoaderData::_claim_other) {}
+  ZVerifyOopClosure(bool verify_weaks) :
+      ClaimMetadataVisitingOopIterateClosure(ClassLoaderData::_claim_other),
+      _verify_weaks(verify_weaks) {}
 
-  virtual void do_oop(oop* p);
-  virtual void do_oop(narrowOop* p) { ShouldNotReachHere(); }
+  virtual void do_oop(oop* p) {
+    if (_verify_weaks) {
+      verify_possibly_weak_oop(p);
+    } else {
+      // We should never encounter finalizable oops through strong
+      // paths. This assumes we have only visited strong roots.
+      verify_oop(p);
+    }
+  }
+
+  virtual void do_oop(narrowOop* p) {
+    ShouldNotReachHere();
+  }
 
   virtual ReferenceIterationMode reference_iteration_mode() {
-    return VisitReferents ? DO_FIELDS : DO_FIELDS_EXCEPT_REFERENT;
+    return _verify_weaks ? DO_FIELDS : DO_FIELDS_EXCEPT_REFERENT;
   }
 
 #ifdef ASSERT
@@ -76,47 +99,37 @@
 #endif
 };
 
-class ZVerifyObjectClosure : public ObjectClosure {
-private:
-  bool _visit_referents;
+template <typename RootsIterator>
+void ZVerify::roots() {
+  assert(SafepointSynchronize::is_at_safepoint(), "Must be at a safepoint");
+  assert(!ZResurrection::is_blocked(), "Invalid phase");
 
-public:
-  ZVerifyObjectClosure(bool visit_referents) : _visit_referents(visit_referents) {}
-  virtual void do_object(oop o);
-};
-
-template <typename RootsIterator>
-void ZVerify::roots_impl() {
   if (ZVerifyRoots) {
-    ZVerifyRootsClosure cl;
+    ZVerifyRootClosure cl;
     RootsIterator iter;
     iter.oops_do(&cl);
   }
 }
 
 void ZVerify::roots_strong() {
-  roots_impl<ZRootsIterator>();
-}
-
-class ZVerifyConcurrentRootsIterator : public ZConcurrentRootsIterator {
-public:
-  ZVerifyConcurrentRootsIterator()
-      : ZConcurrentRootsIterator(ClassLoaderData::_claim_none) {}
-};
-
-void ZVerify::roots_concurrent() {
-  roots_impl<ZVerifyConcurrentRootsIterator>();
+  roots<ZRootsIterator>();
 }
 
 void ZVerify::roots_weak() {
-  assert(!ZResurrection::is_blocked(), "Invalid phase");
+  roots<ZWeakRootsIterator>();
+}
 
-  roots_impl<ZWeakRootsIterator>();
+void ZVerify::roots_concurrent_strong() {
+  roots<ZConcurrentRootsIteratorClaimNone>();
+}
+
+void ZVerify::roots_concurrent_weak() {
+  roots<ZConcurrentWeakRootsIterator>();
 }
 
 void ZVerify::roots(bool verify_weaks) {
   roots_strong();
-  roots_concurrent();
+  roots_concurrent_strong();
   if (verify_weaks) {
     roots_weak();
     roots_concurrent_weak();
@@ -124,21 +137,18 @@
 }
 
 void ZVerify::objects(bool verify_weaks) {
+  assert(SafepointSynchronize::is_at_safepoint(), "Must be at a safepoint");
+  assert(ZGlobalPhase == ZPhaseMarkCompleted, "Invalid phase");
+  assert(!ZResurrection::is_blocked(), "Invalid phase");
+
   if (ZVerifyObjects) {
-    ZVerifyObjectClosure cl(verify_weaks);
-    ZHeap::heap()->object_iterate(&cl, verify_weaks);
+    ZVerifyOopClosure cl(verify_weaks);
+    ObjectToOopClosure object_cl(&cl);
+    ZHeap::heap()->object_iterate(&object_cl, verify_weaks);
   }
 }
 
-void ZVerify::roots_concurrent_weak() {
-  assert(!ZResurrection::is_blocked(), "Invalid phase");
-
-  roots_impl<ZConcurrentWeakRootsIterator>();
-}
-
 void ZVerify::roots_and_objects(bool verify_weaks) {
-  ZStatTimerDisable  _disable;
-
   roots(verify_weaks);
   objects(verify_weaks);
 }
@@ -150,44 +160,13 @@
 }
 
 void ZVerify::after_mark() {
-  // Only verify strong roots and references.
+  // Verify all strong roots and strong references
+  ZStatTimerDisable disable;
   roots_and_objects(false /* verify_weaks */);
 }
 
 void ZVerify::after_weak_processing() {
-  // Also verify weaks - all should have been processed at this point.
+  // Verify all roots and all references
+  ZStatTimerDisable disable;
   roots_and_objects(true /* verify_weaks */);
 }
-
-template <bool VisitReferents>
-void ZVerifyOopClosure<VisitReferents>::do_oop(oop* p) {
-  guarantee(SafepointSynchronize::is_at_safepoint(), "Must be at a safepoint");
-  guarantee(ZGlobalPhase == ZPhaseMarkCompleted, "Invalid phase");
-  guarantee(!ZResurrection::is_blocked(), "Invalid phase");
-
-  const oop o = RawAccess<>::oop_load(p);
-  if (o == NULL) {
-    return;
-  }
-
-  const uintptr_t addr = ZOop::to_address(o);
-  if (VisitReferents) {
-    guarantee(ZAddress::is_good(addr) || ZAddress::is_finalizable_good(addr), BAD_OOP_REPORT(addr));
-  } else {
-    // Should not encounter finalizable oops through strong-only paths. Assumes only strong roots are visited.
-    guarantee(ZAddress::is_good(addr), BAD_OOP_REPORT(addr));
-  }
-
-  const uintptr_t good_addr = ZAddress::good(addr);
-  guarantee(oopDesc::is_oop(ZOop::from_address(good_addr)), BAD_OOP_REPORT(addr));
-}
-
-void ZVerifyObjectClosure::do_object(oop o) {
-  if (_visit_referents) {
-    ZVerifyOopClosure<true /* VisitReferents */> cl;
-    o->oop_iterate((OopIterateClosure*)&cl);
-  } else {
-    ZVerifyOopClosure<false /* VisitReferents */> cl;
-    o->oop_iterate(&cl);
-  }
-}
--- a/src/hotspot/share/gc/z/zVerify.hpp	Tue Aug 06 15:50:00 2019 +0200
+++ b/src/hotspot/share/gc/z/zVerify.hpp	Tue Aug 06 15:50:12 2019 +0200
@@ -28,48 +28,21 @@
 
 class ZVerify : public AllStatic {
 private:
-  template <typename RootsIterator>
-  static void roots_impl();
-  static void roots(bool verify_weaks);
+  template <typename RootsIterator> static void roots();
 
   static void roots_strong();
   static void roots_weak();
-  static void roots_concurrent();
+  static void roots_concurrent_strong();
   static void roots_concurrent_weak();
 
+  static void roots(bool verify_weaks);
   static void objects(bool verify_weaks);
-
-  static void roots_and_objects(bool visit_weaks);
+  static void roots_and_objects(bool verify_weaks);
 
 public:
-  // Verify strong (non-concurrent) roots. Should always be good.
   static void before_zoperation();
-
-  // Verify all strong roots and references after marking.
   static void after_mark();
-
-  // Verify strong and weak roots and references.
   static void after_weak_processing();
 };
 
-class VM_ZVerifyOperation : public VM_Operation {
-public:
-  virtual bool needs_inactive_gc_locker() const {
-    // An inactive GC locker is needed in operations where we change the bad
-    // mask or move objects. Changing the bad mask will invalidate all oops,
-    // which makes it conceptually the same thing as moving all objects.
-    return false;
-  }
-
-  virtual void doit() {
-    ZVerify::after_weak_processing();
-  }
-
-  bool success() const {
-    return true;
-  }
-
-  virtual VMOp_Type type() const { return VMOp_ZVerify; }
-};
-
 #endif // SHARE_GC_Z_ZVERIFY_HPP