# HG changeset patch
# User pliden
# Date 1541141160 -3600
# Node ID 2e280ecec24604e760f500cae85855bd142f09c0
# Parent  7f90bc64b0fccbc86289f25c467d5d9342035da8
8212921: ZGC: Move verification to after resurrection unblocked
Reviewed-by: eosterlund

diff -r 7f90bc64b0fc -r 2e280ecec246 src/hotspot/share/gc/z/zAddress.hpp
--- a/src/hotspot/share/gc/z/zAddress.hpp	Fri Nov 02 00:26:25 2018 -0700
+++ b/src/hotspot/share/gc/z/zAddress.hpp	Fri Nov 02 07:46:00 2018 +0100
@@ -37,6 +37,7 @@
   static bool is_weak_good_or_null(uintptr_t value);
   static bool is_marked(uintptr_t value);
   static bool is_finalizable(uintptr_t value);
+  static bool is_finalizable_good(uintptr_t value);
   static bool is_remapped(uintptr_t value);
 
   static uintptr_t address(uintptr_t value);
diff -r 7f90bc64b0fc -r 2e280ecec246 src/hotspot/share/gc/z/zAddress.inline.hpp
--- a/src/hotspot/share/gc/z/zAddress.inline.hpp	Fri Nov 02 00:26:25 2018 -0700
+++ b/src/hotspot/share/gc/z/zAddress.inline.hpp	Fri Nov 02 07:46:00 2018 +0100
@@ -74,6 +74,10 @@
   return value & ZAddressMetadataFinalizable;
 }
 
+inline bool ZAddress::is_finalizable_good(uintptr_t value) {
+  return is_finalizable(value) && is_good(value ^ ZAddressMetadataFinalizable);
+}
+
 inline bool ZAddress::is_remapped(uintptr_t value) {
   return value & ZAddressMetadataRemapped;
 }
diff -r 7f90bc64b0fc -r 2e280ecec246 src/hotspot/share/gc/z/zDriver.cpp
--- a/src/hotspot/share/gc/z/zDriver.cpp	Fri Nov 02 00:26:25 2018 -0700
+++ b/src/hotspot/share/gc/z/zDriver.cpp	Fri Nov 02 07:46:00 2018 +0100
@@ -33,6 +33,7 @@
 #include "gc/z/zServiceability.hpp"
 #include "gc/z/zStat.hpp"
 #include "logging/log.hpp"
+#include "memory/universe.hpp"
 #include "runtime/vm_operations.hpp"
 #include "runtime/vmThread.hpp"
 
@@ -44,6 +45,7 @@
 static const ZStatPhaseConcurrent ZPhaseConcurrentProcessNonStrongReferences("Concurrent Process Non-Strong References");
 static const ZStatPhaseConcurrent ZPhaseConcurrentResetRelocationSet("Concurrent Reset Relocation Set");
 static const ZStatPhaseConcurrent ZPhaseConcurrentDestroyDetachedPages("Concurrent Destroy Detached Pages");
+static const ZStatPhasePause      ZPhasePauseVerify("Pause Verify");
 static const ZStatPhaseConcurrent ZPhaseConcurrentSelectRelocationSet("Concurrent Select Relocation Set");
 static const ZStatPhaseConcurrent ZPhaseConcurrentPrepareRelocationSet("Concurrent Prepare Relocation Set");
 static const ZStatPhasePause      ZPhasePauseRelocateStart("Pause Relocate Start");
@@ -210,6 +212,19 @@
   }
 };
 
+class ZVerifyClosure : public ZOperationClosure {
+public:
+  virtual const char* name() const {
+    return "ZVerify";
+  }
+
+  virtual bool do_operation() {
+    ZStatTimer timer(ZPhasePauseVerify);
+    Universe::verify();
+    return true;
+  }
+};
+
 class ZRelocateStartClosure : public ZOperationClosure {
 public:
   virtual const char* name() const {
@@ -367,25 +382,31 @@
     ZHeap::heap()->destroy_detached_pages();
   }
 
-  // Phase 7: Concurrent Select Relocation Set
+  // Phase 7: Pause Verify
+  if (VerifyBeforeGC || VerifyDuringGC || VerifyAfterGC) {
+    ZVerifyClosure cl;
+    vm_operation(&cl);
+  }
+
+  // Phase 8: Concurrent Select Relocation Set
   {
     ZStatTimer timer(ZPhaseConcurrentSelectRelocationSet);
     ZHeap::heap()->select_relocation_set();
   }
 
-  // Phase 8: Concurrent Prepare Relocation Set
+  // Phase 9: Concurrent Prepare Relocation Set
   {
     ZStatTimer timer(ZPhaseConcurrentPrepareRelocationSet);
     ZHeap::heap()->prepare_relocation_set();
   }
 
-  // Phase 9: Pause Relocate Start
+  // Phase 10: Pause Relocate Start
   {
     ZRelocateStartClosure cl;
     vm_operation(&cl);
   }
 
-  // Phase 10: Concurrent Relocate
+  // Phase 11: Concurrent Relocate
   {
     ZStatTimer timer(ZPhaseConcurrentRelocated);
     ZHeap::heap()->relocate();
diff -r 7f90bc64b0fc -r 2e280ecec246 src/hotspot/share/gc/z/zHeap.cpp
--- a/src/hotspot/share/gc/z/zHeap.cpp	Fri Nov 02 00:26:25 2018 -0700
+++ b/src/hotspot/share/gc/z/zHeap.cpp	Fri Nov 02 07:46:00 2018 +0100
@@ -366,11 +366,6 @@
   // Process weak roots
   _weak_roots_processor.process_weak_roots();
 
-  // Verification
-  if (VerifyBeforeGC || VerifyDuringGC || VerifyAfterGC) {
-    Universe::verify();
-  }
-
   return true;
 }
 
@@ -570,7 +565,7 @@
       _weak_roots() {}
 
   virtual void work() {
-    ZVerifyRootOopClosure cl;
+    ZVerifyOopClosure cl;
     _strong_roots.oops_do(&cl);
     _weak_roots.oops_do(&cl);
   }
diff -r 7f90bc64b0fc -r 2e280ecec246 src/hotspot/share/gc/z/zOop.hpp
--- a/src/hotspot/share/gc/z/zOop.hpp	Fri Nov 02 00:26:25 2018 -0700
+++ b/src/hotspot/share/gc/z/zOop.hpp	Fri Nov 02 07:46:00 2018 +0100
@@ -33,7 +33,7 @@
   static uintptr_t to_address(oop o);
 
   static bool is_good(oop o);
-  static bool is_good_or_null(oop o);
+  static bool is_finalizable_good(oop o);
 
   static oop good(oop);
 };
diff -r 7f90bc64b0fc -r 2e280ecec246 src/hotspot/share/gc/z/zOop.inline.hpp
--- a/src/hotspot/share/gc/z/zOop.inline.hpp	Fri Nov 02 00:26:25 2018 -0700
+++ b/src/hotspot/share/gc/z/zOop.inline.hpp	Fri Nov 02 07:46:00 2018 +0100
@@ -40,8 +40,8 @@
   return ZAddress::is_good(to_address(o));
 }
 
-inline bool ZOop::is_good_or_null(oop o) {
-  return ZAddress::is_good_or_null(to_address(o));
+inline bool ZOop::is_finalizable_good(oop o) {
+  return ZAddress::is_finalizable_good(to_address(o));
 }
 
 inline oop ZOop::good(oop o) {
diff -r 7f90bc64b0fc -r 2e280ecec246 src/hotspot/share/gc/z/zOopClosures.cpp
--- a/src/hotspot/share/gc/z/zOopClosures.cpp	Fri Nov 02 00:26:25 2018 -0700
+++ b/src/hotspot/share/gc/z/zOopClosures.cpp	Fri Nov 02 07:46:00 2018 +0100
@@ -28,54 +28,31 @@
 #include "memory/iterator.inline.hpp"
 #include "oops/access.inline.hpp"
 #include "oops/oop.inline.hpp"
+#include "runtime/safepoint.hpp"
 #include "utilities/debug.hpp"
 #include "utilities/globalDefinitions.hpp"
 
-static void z_verify_loaded_object(const oop* p, const oop obj) {
-  guarantee(ZOop::is_good_or_null(obj),
-            "Bad oop " PTR_FORMAT " found at " PTR_FORMAT ", expected " PTR_FORMAT,
-            p2i(obj), p2i(p), p2i(ZOop::good(obj)));
-  guarantee(oopDesc::is_oop_or_null(obj),
-            "Bad object " PTR_FORMAT " found at " PTR_FORMAT,
-            p2i(obj), p2i(p));
-}
+void ZVerifyOopClosure::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");
 
-OopIterateClosure::ReferenceIterationMode ZVerifyHeapOopClosure::reference_iteration_mode() {
-  // Don't visit the j.l.Reference.referents for this verification closure,
-  // since they are cleaned concurrently after ZHeap::mark_end(), and can
-  // therefore not be verified at this point.
-  return DO_FIELDS_EXCEPT_REFERENT;
+  const oop o = RawAccess<>::oop_load(p);
+  if (o != NULL) {
+    guarantee(ZOop::is_good(o) || ZOop::is_finalizable_good(o),
+              "Bad oop " PTR_FORMAT " found at " PTR_FORMAT ", expected " PTR_FORMAT,
+              p2i(o), p2i(p), p2i(ZOop::good(o)));
+    guarantee(oopDesc::is_oop(ZOop::good(o)),
+              "Bad object " PTR_FORMAT " found at " PTR_FORMAT,
+              p2i(o), p2i(p));
+  }
 }
 
-void ZVerifyHeapOopClosure::do_oop(oop* p) {
-  guarantee(ZHeap::heap()->is_in((uintptr_t)p), "oop* " PTR_FORMAT " not in heap", p2i(p));
-
-  const oop obj = RawAccess<>::oop_load(p);
-  z_verify_loaded_object(p, obj);
-}
-
-void ZVerifyHeapOopClosure::do_oop(narrowOop* p) {
-  ShouldNotReachHere();
-}
-
-ZVerifyRootOopClosure::ZVerifyRootOopClosure() {
-  // This closure should only be used from ZHeap::mark_end(),
-  // when all roots should have been fixed by the fixup_partial_loads().
-  guarantee(ZGlobalPhase == ZPhaseMarkCompleted, "Invalid phase");
-}
-
-void ZVerifyRootOopClosure::do_oop(oop* p) {
-  guarantee(!ZHeap::heap()->is_in((uintptr_t)p), "oop* " PTR_FORMAT " in heap", p2i(p));
-
-  const oop obj = RawAccess<>::oop_load(p);
-  z_verify_loaded_object(p, obj);
-}
-
-void ZVerifyRootOopClosure::do_oop(narrowOop* p) {
+void ZVerifyOopClosure::do_oop(narrowOop* p) {
   ShouldNotReachHere();
 }
 
 void ZVerifyObjectClosure::do_object(oop o) {
-  ZVerifyHeapOopClosure cl;
+  ZVerifyOopClosure cl;
   o->oop_iterate(&cl);
 }
diff -r 7f90bc64b0fc -r 2e280ecec246 src/hotspot/share/gc/z/zOopClosures.hpp
--- a/src/hotspot/share/gc/z/zOopClosures.hpp	Fri Nov 02 00:26:25 2018 -0700
+++ b/src/hotspot/share/gc/z/zOopClosures.hpp	Fri Nov 02 07:46:00 2018 +0100
@@ -71,29 +71,23 @@
   virtual void do_oop(narrowOop* p);
 };
 
-class ZVerifyHeapOopClosure : public BasicOopIterateClosure {
+class ZVerifyOopClosure : public ZRootsIteratorClosure, public BasicOopIterateClosure {
 public:
-  virtual ReferenceIterationMode reference_iteration_mode();
-
   virtual void do_oop(oop* p);
   virtual void do_oop(narrowOop* p);
 
+  virtual ReferenceIterationMode reference_iteration_mode() {
+    return DO_FIELDS;
+  }
+
 #ifdef ASSERT
-  // Verification handled by the closure itself.
+  // Verification handled by the closure itself
   virtual bool should_verify_oops() {
     return false;
   }
 #endif
 };
 
-class ZVerifyRootOopClosure : public ZRootsIteratorClosure {
-public:
-  ZVerifyRootOopClosure();
-
-  virtual void do_oop(oop* p);
-  virtual void do_oop(narrowOop* p);
-};
-
 class ZVerifyObjectClosure : public ObjectClosure {
 public:
   virtual void do_object(oop o);