8329858: G1: Make G1VerifyLiveAndRemSetClosure stateless

Reviewed-by: ayang, tschatzl

src/hotspot/share/gc/g1/g1HeapRegion.cpp

@@ -462,17 +462,31 @@ static bool is_oop_safe(oop obj) {
   return true;
 }
 
-// Closure that glues together validity check for oop references (first),
+class G1VerifyFailureCounter {
-// then optionally verifies the remembered set for that reference.
+  size_t _count;
-class G1VerifyLiveAndRemSetClosure : public BasicOopIterateClosure {
+
-  VerifyOption _vo;
+public:
-  oop _containing_obj;
+  G1VerifyFailureCounter() : _count(0) {}
-  size_t _num_failures;
 
   // Increases the failure counter and return whether this has been the first failure.
   bool record_failure() {
-    _num_failures++;
+    _count++;
-    return _num_failures == 1;
+    return _count == 1;
+  }
+
+  size_t count() const { return _count; }
+};
+
+// Closure that glues together validity check for oop references (first),
+// then optionally verifies the remembered set for that reference.
+class G1VerifyLiveAndRemSetClosure : public BasicOopIterateClosure {
+  const VerifyOption _vo;
+  const oop _containing_obj;
+  G1VerifyFailureCounter* const _failures;
+
+  // Increases the failure counter and return whether this has been the first failure.
+  bool record_failure() {
+    return _failures->record_failure();
   }
 
   static void print_object(outputStream* out, oop obj) {
@@ -486,18 +500,22 @@ class G1VerifyLiveAndRemSetClosure : public BasicOopIterateClosure {
   template <class T>
   struct Checker {
     G1CollectedHeap* _g1h;
-    G1VerifyLiveAndRemSetClosure* _cl;
+    G1VerifyFailureCounter* _failures;
     oop _containing_obj;
     T* _p;
     oop _obj;
 
-    Checker(G1VerifyLiveAndRemSetClosure* cl, oop containing_obj, T* p, oop obj) :
+    Checker(G1VerifyFailureCounter* failures, oop containing_obj, T* p, oop obj) :
       _g1h(G1CollectedHeap::heap()),
-      _cl(cl),
+      _failures(failures),
       _containing_obj(containing_obj),
       _p(p),
       _obj(obj) { }
 
+    bool record_failure() {
+      return _failures->record_failure();
+    }
+
     void print_containing_obj(outputStream* out, HeapRegion* from) {
       log_error(gc, verify)("Field " PTR_FORMAT " of obj " PTR_FORMAT " in region " HR_FORMAT,
                             p2i(_p), p2i(_containing_obj), HR_FORMAT_PARAMS(from));
@@ -516,7 +534,8 @@ class G1VerifyLiveAndRemSetClosure : public BasicOopIterateClosure {
     VerifyOption _vo;
     bool _is_in_heap;
 
-    LiveChecker(G1VerifyLiveAndRemSetClosure* cl, oop containing_obj, T* p, oop obj, VerifyOption vo) : Checker<T>(cl, containing_obj, p, obj) {
+    LiveChecker(G1VerifyFailureCounter* failures, oop containing_obj, T* p, oop obj, VerifyOption vo)
+      : Checker<T>(failures, containing_obj, p, obj) {
       _vo = vo;
       _is_in_heap = this->_g1h->is_in(obj);
     }
@@ -532,7 +551,7 @@ class G1VerifyLiveAndRemSetClosure : public BasicOopIterateClosure {
 
       MutexLocker x(G1RareEvent_lock, Mutex::_no_safepoint_check_flag);
 
-      if (this->_cl->record_failure()) {
+      if (this->record_failure()) {
         log.error("----------");
       }
 
@@ -558,7 +577,8 @@ class G1VerifyLiveAndRemSetClosure : public BasicOopIterateClosure {
     CardValue _cv_obj;
     CardValue _cv_field;
 
-    RemSetChecker(G1VerifyLiveAndRemSetClosure* cl, oop containing_obj, T* p, oop obj) : Checker<T>(cl, containing_obj, p, obj) {
+    RemSetChecker(G1VerifyFailureCounter* failures, oop containing_obj, T* p, oop obj)
+      : Checker<T>(failures, containing_obj, p, obj) {
       _from = this->_g1h->heap_region_containing(p);
       _to = this->_g1h->heap_region_containing(obj);
 
@@ -585,7 +605,7 @@ class G1VerifyLiveAndRemSetClosure : public BasicOopIterateClosure {
 
       MutexLocker x(G1RareEvent_lock, Mutex::_no_safepoint_check_flag);
 
-      if (this->_cl->record_failure()) {
+      if (this->record_failure()) {
         log.error("----------");
       }
       log.error("Missing rem set entry:");
@@ -598,9 +618,7 @@ class G1VerifyLiveAndRemSetClosure : public BasicOopIterateClosure {
 
   template <class T>
   void do_oop_work(T* p) {
-    assert(_containing_obj != nullptr, "must be");
+    if (_failures->count() >= G1MaxVerifyFailures) {
-
-    if (num_failures() >= G1MaxVerifyFailures) {
       return;
     }
 
@@ -610,31 +628,24 @@ class G1VerifyLiveAndRemSetClosure : public BasicOopIterateClosure {
     }
     oop obj = CompressedOops::decode_raw_not_null(heap_oop);
 
-    LiveChecker<T> live_check(this, _containing_obj, p, obj, _vo);
+    LiveChecker<T> live_check(_failures, _containing_obj, p, obj, _vo);
     if (live_check.failed()) {
       live_check.report_error();
       // There is no point in doing remset verification if the reference is bad.
       return;
     }
 
-    RemSetChecker<T> remset_check(this, _containing_obj, p, obj);
+    RemSetChecker<T> remset_check(_failures, _containing_obj, p, obj);
     if (remset_check.failed()) {
       remset_check.report_error();
     }
   }
 
 public:
-  G1VerifyLiveAndRemSetClosure(G1CollectedHeap* g1h, VerifyOption vo) :
+  G1VerifyLiveAndRemSetClosure(oop containing_obj, VerifyOption vo, G1VerifyFailureCounter* failures)
-    _vo(vo),
+    : _vo(vo),
-    _containing_obj(nullptr),
+      _containing_obj(containing_obj),
-    _num_failures(0) { }
+      _failures(failures) {}
-
-  void set_containing_obj(oop const obj) {
-    assert(!G1CollectedHeap::heap()->is_obj_dead_cond(obj, _vo), "Precondition");
-    _containing_obj = obj;
-  }
-
-  size_t num_failures() const { return _num_failures; }
 
   virtual inline void do_oop(narrowOop* p) { do_oop_work(p); }
   virtual inline void do_oop(oop* p) { do_oop_work(p); }
@@ -643,9 +654,7 @@ public:
 bool HeapRegion::verify_liveness_and_remset(VerifyOption vo) const {
   G1CollectedHeap* g1h = G1CollectedHeap::heap();
 
-  G1VerifyLiveAndRemSetClosure cl(g1h, vo);
+  G1VerifyFailureCounter failures;
-
-  size_t other_failures = 0;
 
   HeapWord* p;
   for (p = bottom(); p < top(); p += block_size(p)) {
@@ -656,13 +665,13 @@ bool HeapRegion::verify_liveness_and_remset(VerifyOption vo) const {
     }
 
     if (is_oop_safe(obj)) {
-      cl.set_containing_obj(obj);
+      G1VerifyLiveAndRemSetClosure cl(obj, vo, &failures);
       obj->oop_iterate(&cl);
     } else {
-      other_failures++;
+      failures.record_failure();
     }
 
-    if ((cl.num_failures() + other_failures) >= G1MaxVerifyFailures) {
+    if (failures.count() >= G1MaxVerifyFailures) {
       return true;
     }
   }
@@ -672,7 +681,7 @@ bool HeapRegion::verify_liveness_and_remset(VerifyOption vo) const {
                           p2i(p), p2i(top()));
     return true;
   }
-  return (cl.num_failures() + other_failures) != 0;
+  return failures.count() != 0;
 }
 
 bool HeapRegion::verify(VerifyOption vo) const {