From 5c9f03686d77dd56c0441f9eb54b1cfcd3a49b3c Mon Sep 17 00:00:00 2001 From: Stefan Karlsson Date: Tue, 9 Apr 2024 12:32:17 +0000 Subject: [PATCH] 8329858: G1: Make G1VerifyLiveAndRemSetClosure stateless Reviewed-by: ayang, tschatzl --- src/hotspot/share/gc/g1/g1HeapRegion.cpp | 85 +++++++++++++----------- 1 file changed, 47 insertions(+), 38 deletions(-) diff --git a/src/hotspot/share/gc/g1/g1HeapRegion.cpp b/src/hotspot/share/gc/g1/g1HeapRegion.cpp index 55280972017..b1fca6bc306 100644 --- a/src/hotspot/share/gc/g1/g1HeapRegion.cpp +++ b/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), -// then optionally verifies the remembered set for that reference. -class G1VerifyLiveAndRemSetClosure : public BasicOopIterateClosure { - VerifyOption _vo; - oop _containing_obj; - size_t _num_failures; +class G1VerifyFailureCounter { + size_t _count; + +public: + G1VerifyFailureCounter() : _count(0) {} // Increases the failure counter and return whether this has been the first failure. bool record_failure() { - _num_failures++; - return _num_failures == 1; + _count++; + 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 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(cl, containing_obj, p, obj) { + LiveChecker(G1VerifyFailureCounter* failures, oop containing_obj, T* p, oop obj, VerifyOption vo) + : Checker(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(cl, containing_obj, p, obj) { + RemSetChecker(G1VerifyFailureCounter* failures, oop containing_obj, T* p, oop obj) + : Checker(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 void do_oop_work(T* p) { - assert(_containing_obj != nullptr, "must be"); - - if (num_failures() >= G1MaxVerifyFailures) { + if (_failures->count() >= G1MaxVerifyFailures) { return; } @@ -610,31 +628,24 @@ class G1VerifyLiveAndRemSetClosure : public BasicOopIterateClosure { } oop obj = CompressedOops::decode_raw_not_null(heap_oop); - LiveChecker live_check(this, _containing_obj, p, obj, _vo); + LiveChecker 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 remset_check(this, _containing_obj, p, obj); + RemSetChecker remset_check(_failures, _containing_obj, p, obj); if (remset_check.failed()) { remset_check.report_error(); } } public: - G1VerifyLiveAndRemSetClosure(G1CollectedHeap* g1h, VerifyOption vo) : - _vo(vo), - _containing_obj(nullptr), - _num_failures(0) { } - - 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; } + G1VerifyLiveAndRemSetClosure(oop containing_obj, VerifyOption vo, G1VerifyFailureCounter* failures) + : _vo(vo), + _containing_obj(containing_obj), + _failures(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); - - size_t other_failures = 0; + G1VerifyFailureCounter failures; 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 {