From 0e040a455811ac698f6d300c8015bb55dee226d6 Mon Sep 17 00:00:00 2001 From: johnc Date: Tue, 14 Jun 2011 11:01:10 -0700 Subject: [PATCH] 7004681: G1: Extend marking verification to Full GCs Summary: Perform a heap verification after the first phase of G1's full GC using objects' mark words to determine liveness. The third parameter of the heap verification routines, which was used in G1 to determine which marking bitmap to use in liveness calculations, has been changed from a boolean to an enum with values defined for using the mark word, and the 'prev' and 'next' bitmaps. Reviewed-by: tonyp, ysr --- .../gc_implementation/g1/concurrentMark.cpp | 156 ++++++++++------ .../gc_implementation/g1/concurrentMark.hpp | 16 +- .../gc_implementation/g1/g1CollectedHeap.cpp | 176 ++++++++++-------- .../gc_implementation/g1/g1CollectedHeap.hpp | 67 ++++--- .../vm/gc_implementation/g1/g1MarkSweep.cpp | 28 ++- .../vm/gc_implementation/g1/heapRegion.cpp | 23 +-- .../vm/gc_implementation/g1/heapRegion.hpp | 18 +- .../parallelScavenge/parallelScavengeHeap.cpp | 2 +- .../parallelScavenge/parallelScavengeHeap.hpp | 2 +- src/share/vm/gc_interface/collectedHeap.hpp | 2 +- src/share/vm/memory/genCollectedHeap.cpp | 2 +- src/share/vm/memory/genCollectedHeap.hpp | 2 +- src/share/vm/memory/universe.cpp | 2 +- src/share/vm/memory/universe.hpp | 11 +- 14 files changed, 309 insertions(+), 198 deletions(-) diff --git a/src/share/vm/gc_implementation/g1/concurrentMark.cpp b/src/share/vm/gc_implementation/g1/concurrentMark.cpp index 9e485b39d..b45c1b52a 100644 --- a/src/share/vm/gc_implementation/g1/concurrentMark.cpp +++ b/src/share/vm/gc_implementation/g1/concurrentMark.cpp @@ -782,7 +782,7 @@ void ConcurrentMark::checkpointRootsInitialPre() { #ifndef PRODUCT if (G1PrintReachableAtInitialMark) { print_reachable("at-cycle-start", - true /* use_prev_marking */, true /* all */); + VerifyOption_G1UsePrevMarking, true /* all */); } #endif @@ -1200,7 +1200,9 @@ void ConcurrentMark::checkpointRootsFinal(bool clear_all_soft_refs) { HandleMark hm; // handle scope gclog_or_tty->print(" VerifyDuringGC:(before)"); Universe::heap()->prepare_for_verify(); - Universe::verify(true, false, true); + Universe::verify(/* allow dirty */ true, + /* silent */ false, + /* option */ VerifyOption_G1UsePrevMarking); } G1CollectorPolicy* g1p = g1h->g1_policy(); @@ -1233,9 +1235,9 @@ void ConcurrentMark::checkpointRootsFinal(bool clear_all_soft_refs) { HandleMark hm; // handle scope gclog_or_tty->print(" VerifyDuringGC:(after)"); Universe::heap()->prepare_for_verify(); - Universe::heap()->verify(/* allow_dirty */ true, - /* silent */ false, - /* use_prev_marking */ false); + Universe::verify(/* allow dirty */ true, + /* silent */ false, + /* option */ VerifyOption_G1UseNextMarking); } assert(!restart_for_overflow(), "sanity"); } @@ -1725,9 +1727,9 @@ void ConcurrentMark::cleanup() { HandleMark hm; // handle scope gclog_or_tty->print(" VerifyDuringGC:(before)"); Universe::heap()->prepare_for_verify(); - Universe::verify(/* allow dirty */ true, - /* silent */ false, - /* prev marking */ true); + Universe::verify(/* allow dirty */ true, + /* silent */ false, + /* option */ VerifyOption_G1UsePrevMarking); } G1CollectorPolicy* g1p = G1CollectedHeap::heap()->g1_policy(); @@ -1873,9 +1875,9 @@ void ConcurrentMark::cleanup() { HandleMark hm; // handle scope gclog_or_tty->print(" VerifyDuringGC:(after)"); Universe::heap()->prepare_for_verify(); - Universe::verify(/* allow dirty */ true, - /* silent */ false, - /* prev marking */ true); + Universe::verify(/* allow dirty */ true, + /* silent */ false, + /* option */ VerifyOption_G1UsePrevMarking); } g1h->verify_region_sets_optional(); @@ -2344,18 +2346,16 @@ void ConcurrentMark::checkpointRootsFinalWork() { class PrintReachableOopClosure: public OopClosure { private: G1CollectedHeap* _g1h; - CMBitMapRO* _bitmap; outputStream* _out; - bool _use_prev_marking; + VerifyOption _vo; bool _all; public: - PrintReachableOopClosure(CMBitMapRO* bitmap, - outputStream* out, - bool use_prev_marking, + PrintReachableOopClosure(outputStream* out, + VerifyOption vo, bool all) : _g1h(G1CollectedHeap::heap()), - _bitmap(bitmap), _out(out), _use_prev_marking(use_prev_marking), _all(all) { } + _out(out), _vo(vo), _all(all) { } void do_oop(narrowOop* p) { do_oop_work(p); } void do_oop( oop* p) { do_oop_work(p); } @@ -2373,12 +2373,23 @@ public: HeapRegion* hr = _g1h->heap_region_containing(obj); guarantee(hr != NULL, "invariant"); bool over_tams = false; - if (_use_prev_marking) { - over_tams = hr->obj_allocated_since_prev_marking(obj); - } else { - over_tams = hr->obj_allocated_since_next_marking(obj); + bool marked = false; + + switch (_vo) { + case VerifyOption_G1UsePrevMarking: + over_tams = hr->obj_allocated_since_prev_marking(obj); + marked = _g1h->isMarkedPrev(obj); + break; + case VerifyOption_G1UseNextMarking: + over_tams = hr->obj_allocated_since_next_marking(obj); + marked = _g1h->isMarkedNext(obj); + break; + case VerifyOption_G1UseMarkWord: + marked = obj->is_gc_marked(); + break; + default: + ShouldNotReachHere(); } - bool marked = _bitmap->isMarked((HeapWord*) obj); if (over_tams) { str = " >"; @@ -2399,35 +2410,45 @@ public: class PrintReachableObjectClosure : public ObjectClosure { private: - CMBitMapRO* _bitmap; - outputStream* _out; - bool _use_prev_marking; - bool _all; - HeapRegion* _hr; + G1CollectedHeap* _g1h; + outputStream* _out; + VerifyOption _vo; + bool _all; + HeapRegion* _hr; public: - PrintReachableObjectClosure(CMBitMapRO* bitmap, - outputStream* out, - bool use_prev_marking, + PrintReachableObjectClosure(outputStream* out, + VerifyOption vo, bool all, HeapRegion* hr) : - _bitmap(bitmap), _out(out), - _use_prev_marking(use_prev_marking), _all(all), _hr(hr) { } + _g1h(G1CollectedHeap::heap()), + _out(out), _vo(vo), _all(all), _hr(hr) { } void do_object(oop o) { - bool over_tams; - if (_use_prev_marking) { - over_tams = _hr->obj_allocated_since_prev_marking(o); - } else { - over_tams = _hr->obj_allocated_since_next_marking(o); + bool over_tams = false; + bool marked = false; + + switch (_vo) { + case VerifyOption_G1UsePrevMarking: + over_tams = _hr->obj_allocated_since_prev_marking(o); + marked = _g1h->isMarkedPrev(o); + break; + case VerifyOption_G1UseNextMarking: + over_tams = _hr->obj_allocated_since_next_marking(o); + marked = _g1h->isMarkedNext(o); + break; + case VerifyOption_G1UseMarkWord: + marked = o->is_gc_marked(); + break; + default: + ShouldNotReachHere(); } - bool marked = _bitmap->isMarked((HeapWord*) o); bool print_it = _all || over_tams || marked; if (print_it) { _out->print_cr(" "PTR_FORMAT"%s", o, (over_tams) ? " >" : (marked) ? " M" : ""); - PrintReachableOopClosure oopCl(_bitmap, _out, _use_prev_marking, _all); + PrintReachableOopClosure oopCl(_out, _vo, _all); o->oop_iterate(&oopCl); } } @@ -2435,9 +2456,8 @@ public: class PrintReachableRegionClosure : public HeapRegionClosure { private: - CMBitMapRO* _bitmap; outputStream* _out; - bool _use_prev_marking; + VerifyOption _vo; bool _all; public: @@ -2446,10 +2466,21 @@ public: HeapWord* e = hr->end(); HeapWord* t = hr->top(); HeapWord* p = NULL; - if (_use_prev_marking) { - p = hr->prev_top_at_mark_start(); - } else { - p = hr->next_top_at_mark_start(); + + switch (_vo) { + case VerifyOption_G1UsePrevMarking: + p = hr->prev_top_at_mark_start(); + break; + case VerifyOption_G1UseNextMarking: + p = hr->next_top_at_mark_start(); + break; + case VerifyOption_G1UseMarkWord: + // When we are verifying marking using the mark word + // TAMS has no relevance. + assert(p == NULL, "post-condition"); + break; + default: + ShouldNotReachHere(); } _out->print_cr("** ["PTR_FORMAT", "PTR_FORMAT"] top: "PTR_FORMAT" " "TAMS: "PTR_FORMAT, b, e, t, p); @@ -2461,8 +2492,7 @@ public: if (to > from) { _out->print_cr("Objects in ["PTR_FORMAT", "PTR_FORMAT"]", from, to); _out->cr(); - PrintReachableObjectClosure ocl(_bitmap, _out, - _use_prev_marking, _all, hr); + PrintReachableObjectClosure ocl(_out, _vo, _all, hr); hr->object_iterate_mem_careful(MemRegion(from, to), &ocl); _out->cr(); } @@ -2470,15 +2500,25 @@ public: return false; } - PrintReachableRegionClosure(CMBitMapRO* bitmap, - outputStream* out, - bool use_prev_marking, + PrintReachableRegionClosure(outputStream* out, + VerifyOption vo, bool all) : - _bitmap(bitmap), _out(out), _use_prev_marking(use_prev_marking), _all(all) { } + _out(out), _vo(vo), _all(all) { } }; +static const char* verify_option_to_tams(VerifyOption vo) { + switch (vo) { + case VerifyOption_G1UsePrevMarking: + return "PTAMS"; + case VerifyOption_G1UseNextMarking: + return "NTAMS"; + default: + return "NONE"; + } +} + void ConcurrentMark::print_reachable(const char* str, - bool use_prev_marking, + VerifyOption vo, bool all) { gclog_or_tty->cr(); gclog_or_tty->print_cr("== Doing heap dump... "); @@ -2505,20 +2545,12 @@ void ConcurrentMark::print_reachable(const char* str, } outputStream* out = &fout; - - CMBitMapRO* bitmap = NULL; - if (use_prev_marking) { - bitmap = _prevMarkBitMap; - } else { - bitmap = _nextMarkBitMap; - } - - out->print_cr("-- USING %s", (use_prev_marking) ? "PTAMS" : "NTAMS"); + out->print_cr("-- USING %s", verify_option_to_tams(vo)); out->cr(); out->print_cr("--- ITERATING OVER REGIONS"); out->cr(); - PrintReachableRegionClosure rcl(bitmap, out, use_prev_marking, all); + PrintReachableRegionClosure rcl(out, vo, all); _g1h->heap_region_iterate(&rcl); out->cr(); diff --git a/src/share/vm/gc_implementation/g1/concurrentMark.hpp b/src/share/vm/gc_implementation/g1/concurrentMark.hpp index 1957670fc..87a9cda74 100644 --- a/src/share/vm/gc_implementation/g1/concurrentMark.hpp +++ b/src/share/vm/gc_implementation/g1/concurrentMark.hpp @@ -736,12 +736,14 @@ public: // will dump the contents of its reference fields, as well as // liveness information for the object and its referents. The dump // will be written to a file with the following name: - // G1PrintReachableBaseFile + "." + str. use_prev_marking decides - // whether the prev (use_prev_marking == true) or next - // (use_prev_marking == false) marking information will be used to - // determine the liveness of each object / referent. If all is true, - // all objects in the heap will be dumped, otherwise only the live - // ones. In the dump the following symbols / abbreviations are used: + // G1PrintReachableBaseFile + "." + str. + // vo decides whether the prev (vo == UsePrevMarking), the next + // (vo == UseNextMarking) marking information, or the mark word + // (vo == UseMarkWord) will be used to determine the liveness of + // each object / referent. + // If all is true, all objects in the heap will be dumped, otherwise + // only the live ones. In the dump the following symbols / breviations + // are used: // M : an explicitly live object (its bitmap bit is set) // > : an implicitly live object (over tams) // O : an object outside the G1 heap (typically: in the perm gen) @@ -749,7 +751,7 @@ public: // AND MARKED : indicates that an object is both explicitly and // implicitly live (it should be one or the other, not both) void print_reachable(const char* str, - bool use_prev_marking, bool all) PRODUCT_RETURN; + VerifyOption vo, bool all) PRODUCT_RETURN; // Clear the next marking bitmap (will be called concurrently). void clearNextBitmap(); diff --git a/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp b/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp index ef7fdd670..8d6669c2a 100644 --- a/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp +++ b/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp @@ -1211,7 +1211,10 @@ bool G1CollectedHeap::do_collection(bool explicit_gc, HandleMark hm; // Discard invalid handles created during verification gclog_or_tty->print(" VerifyBeforeGC:"); prepare_for_verify(); - Universe::verify(true); + Universe::verify(/* allow dirty */ true, + /* silent */ false, + /* option */ VerifyOption_G1UsePrevMarking); + } COMPILER2_PRESENT(DerivedPointerTable::clear()); @@ -1263,7 +1266,6 @@ bool G1CollectedHeap::do_collection(bool explicit_gc, ref_processor()->enable_discovery(); ref_processor()->setup_policy(do_clear_all_soft_refs); - // Do collection work { HandleMark hm; // Discard invalid handles created during gc @@ -1284,7 +1286,10 @@ bool G1CollectedHeap::do_collection(bool explicit_gc, HandleMark hm; // Discard invalid handles created during verification gclog_or_tty->print(" VerifyAfterGC:"); prepare_for_verify(); - Universe::verify(false); + Universe::verify(/* allow dirty */ false, + /* silent */ false, + /* option */ VerifyOption_G1UsePrevMarking); + } NOT_PRODUCT(ref_processor()->verify_no_references_recorded()); @@ -2639,17 +2644,18 @@ void G1CollectedHeap::prepare_for_verify() { } class VerifyLivenessOopClosure: public OopClosure { - G1CollectedHeap* g1h; + G1CollectedHeap* _g1h; + VerifyOption _vo; public: - VerifyLivenessOopClosure(G1CollectedHeap* _g1h) { - g1h = _g1h; - } + VerifyLivenessOopClosure(G1CollectedHeap* g1h, VerifyOption vo): + _g1h(g1h), _vo(vo) + { } void do_oop(narrowOop *p) { do_oop_work(p); } void do_oop( oop *p) { do_oop_work(p); } template void do_oop_work(T *p) { oop obj = oopDesc::load_decode_heap_oop(p); - guarantee(obj == NULL || !g1h->is_obj_dead(obj), + guarantee(obj == NULL || !_g1h->is_obj_dead_cond(obj, _vo), "Dead object referenced by a not dead object"); } }; @@ -2659,18 +2665,30 @@ private: G1CollectedHeap* _g1h; size_t _live_bytes; HeapRegion *_hr; - bool _use_prev_marking; + VerifyOption _vo; public: - // use_prev_marking == true -> use "prev" marking information, - // use_prev_marking == false -> use "next" marking information - VerifyObjsInRegionClosure(HeapRegion *hr, bool use_prev_marking) - : _live_bytes(0), _hr(hr), _use_prev_marking(use_prev_marking) { + // _vo == UsePrevMarking -> use "prev" marking information, + // _vo == UseNextMarking -> use "next" marking information, + // _vo == UseMarkWord -> use mark word from object header. + VerifyObjsInRegionClosure(HeapRegion *hr, VerifyOption vo) + : _live_bytes(0), _hr(hr), _vo(vo) { _g1h = G1CollectedHeap::heap(); } void do_object(oop o) { - VerifyLivenessOopClosure isLive(_g1h); + VerifyLivenessOopClosure isLive(_g1h, _vo); assert(o != NULL, "Huh?"); - if (!_g1h->is_obj_dead_cond(o, _use_prev_marking)) { + if (!_g1h->is_obj_dead_cond(o, _vo)) { + // If the object is alive according to the mark word, + // then verify that the marking information agrees. + // Note we can't verify the contra-positive of the + // above: if the object is dead (according to the mark + // word), it may not be marked, or may have been marked + // but has since became dead, or may have been allocated + // since the last marking. + if (_vo == VerifyOption_G1UseMarkWord) { + guarantee(!_g1h->is_obj_dead(o), "mark word and concurrent mark mismatch"); + } + o->oop_iterate(&isLive); if (!_hr->obj_allocated_since_prev_marking(o)) { size_t obj_size = o->size(); // Make sure we don't overflow @@ -2712,17 +2730,18 @@ public: class VerifyRegionClosure: public HeapRegionClosure { private: - bool _allow_dirty; - bool _par; - bool _use_prev_marking; - bool _failures; + bool _allow_dirty; + bool _par; + VerifyOption _vo; + bool _failures; public: - // use_prev_marking == true -> use "prev" marking information, - // use_prev_marking == false -> use "next" marking information - VerifyRegionClosure(bool allow_dirty, bool par, bool use_prev_marking) + // _vo == UsePrevMarking -> use "prev" marking information, + // _vo == UseNextMarking -> use "next" marking information, + // _vo == UseMarkWord -> use mark word from object header. + VerifyRegionClosure(bool allow_dirty, bool par, VerifyOption vo) : _allow_dirty(allow_dirty), _par(par), - _use_prev_marking(use_prev_marking), + _vo(vo), _failures(false) {} bool failures() { @@ -2734,11 +2753,11 @@ public: "Should be unclaimed at verify points."); if (!r->continuesHumongous()) { bool failures = false; - r->verify(_allow_dirty, _use_prev_marking, &failures); + r->verify(_allow_dirty, _vo, &failures); if (failures) { _failures = true; } else { - VerifyObjsInRegionClosure not_dead_yet_cl(r, _use_prev_marking); + VerifyObjsInRegionClosure not_dead_yet_cl(r, _vo); r->object_iterate(¬_dead_yet_cl); if (r->max_live_bytes() < not_dead_yet_cl.live_bytes()) { gclog_or_tty->print_cr("["PTR_FORMAT","PTR_FORMAT"] " @@ -2758,14 +2777,15 @@ public: class VerifyRootsClosure: public OopsInGenClosure { private: G1CollectedHeap* _g1h; - bool _use_prev_marking; + VerifyOption _vo; bool _failures; public: - // use_prev_marking == true -> use "prev" marking information, - // use_prev_marking == false -> use "next" marking information - VerifyRootsClosure(bool use_prev_marking) : + // _vo == UsePrevMarking -> use "prev" marking information, + // _vo == UseNextMarking -> use "next" marking information, + // _vo == UseMarkWord -> use mark word from object header. + VerifyRootsClosure(VerifyOption vo) : _g1h(G1CollectedHeap::heap()), - _use_prev_marking(use_prev_marking), + _vo(vo), _failures(false) { } bool failures() { return _failures; } @@ -2774,9 +2794,12 @@ public: T heap_oop = oopDesc::load_heap_oop(p); if (!oopDesc::is_null(heap_oop)) { oop obj = oopDesc::decode_heap_oop_not_null(heap_oop); - if (_g1h->is_obj_dead_cond(obj, _use_prev_marking)) { + if (_g1h->is_obj_dead_cond(obj, _vo)) { gclog_or_tty->print_cr("Root location "PTR_FORMAT" " "points to dead obj "PTR_FORMAT, p, (void*) obj); + if (_vo == VerifyOption_G1UseMarkWord) { + gclog_or_tty->print_cr(" Mark word: "PTR_FORMAT, (void*)(obj->mark())); + } obj->print_on(gclog_or_tty); _failures = true; } @@ -2792,19 +2815,19 @@ public: class G1ParVerifyTask: public AbstractGangTask { private: G1CollectedHeap* _g1h; - bool _allow_dirty; - bool _use_prev_marking; - bool _failures; + bool _allow_dirty; + VerifyOption _vo; + bool _failures; public: - // use_prev_marking == true -> use "prev" marking information, - // use_prev_marking == false -> use "next" marking information - G1ParVerifyTask(G1CollectedHeap* g1h, bool allow_dirty, - bool use_prev_marking) : + // _vo == UsePrevMarking -> use "prev" marking information, + // _vo == UseNextMarking -> use "next" marking information, + // _vo == UseMarkWord -> use mark word from object header. + G1ParVerifyTask(G1CollectedHeap* g1h, bool allow_dirty, VerifyOption vo) : AbstractGangTask("Parallel verify task"), _g1h(g1h), _allow_dirty(allow_dirty), - _use_prev_marking(use_prev_marking), + _vo(vo), _failures(false) { } bool failures() { @@ -2813,7 +2836,7 @@ public: void work(int worker_i) { HandleMark hm; - VerifyRegionClosure blk(_allow_dirty, true, _use_prev_marking); + VerifyRegionClosure blk(_allow_dirty, true, _vo); _g1h->heap_region_par_iterate_chunked(&blk, worker_i, HeapRegion::ParVerifyClaimValue); if (blk.failures()) { @@ -2823,19 +2846,21 @@ public: }; void G1CollectedHeap::verify(bool allow_dirty, bool silent) { - verify(allow_dirty, silent, /* use_prev_marking */ true); + verify(allow_dirty, silent, VerifyOption_G1UsePrevMarking); } void G1CollectedHeap::verify(bool allow_dirty, bool silent, - bool use_prev_marking) { + VerifyOption vo) { if (SafepointSynchronize::is_at_safepoint() || ! UseTLAB) { if (!silent) { gclog_or_tty->print("Roots (excluding permgen) "); } - VerifyRootsClosure rootsCl(use_prev_marking); + VerifyRootsClosure rootsCl(vo); CodeBlobToOopClosure blobsCl(&rootsCl, /*do_marking=*/ false); + // We apply the relevant closures to all the oops in the // system dictionary, the string table and the code cache. const int so = SharedHeap::SO_AllClasses | SharedHeap::SO_Strings | SharedHeap::SO_CodeCache; + process_strong_roots(true, // activate StrongRootsScope true, // we set "collecting perm gen" to true, // so we don't reset the dirty cards in the perm gen. @@ -2843,21 +2868,37 @@ void G1CollectedHeap::verify(bool allow_dirty, &rootsCl, &blobsCl, &rootsCl); - // Since we used "collecting_perm_gen" == true above, we will not have - // checked the refs from perm into the G1-collected heap. We check those - // references explicitly below. Whether the relevant cards are dirty - // is checked further below in the rem set verification. - if (!silent) { gclog_or_tty->print("Permgen roots "); } - perm_gen()->oop_iterate(&rootsCl); + + // If we're verifying after the marking phase of a Full GC then we can't + // treat the perm gen as roots into the G1 heap. Some of the objects in + // the perm gen may be dead and hence not marked. If one of these dead + // objects is considered to be a root then we may end up with a false + // "Root location points to dead ob " failure. + if (vo != VerifyOption_G1UseMarkWord) { + // Since we used "collecting_perm_gen" == true above, we will not have + // checked the refs from perm into the G1-collected heap. We check those + // references explicitly below. Whether the relevant cards are dirty + // is checked further below in the rem set verification. + if (!silent) { gclog_or_tty->print("Permgen roots "); } + perm_gen()->oop_iterate(&rootsCl); + } bool failures = rootsCl.failures(); - if (!silent) { gclog_or_tty->print("HeapRegionSets "); } - verify_region_sets(); + + if (vo != VerifyOption_G1UseMarkWord) { + // If we're verifying during a full GC then the region sets + // will have been torn down at the start of the GC. Therefore + // verifying the region sets will fail. So we only verify + // the region sets when not in a full GC. + if (!silent) { gclog_or_tty->print("HeapRegionSets "); } + verify_region_sets(); + } + if (!silent) { gclog_or_tty->print("HeapRegions "); } if (GCParallelVerificationEnabled && ParallelGCThreads > 1) { assert(check_heap_region_claim_values(HeapRegion::InitialClaimValue), "sanity check"); - G1ParVerifyTask task(this, allow_dirty, use_prev_marking); + G1ParVerifyTask task(this, allow_dirty, vo); int n_workers = workers()->total_workers(); set_par_threads(n_workers); workers()->run_task(&task); @@ -2874,7 +2915,7 @@ void G1CollectedHeap::verify(bool allow_dirty, assert(check_heap_region_claim_values(HeapRegion::InitialClaimValue), "sanity check"); } else { - VerifyRegionClosure blk(allow_dirty, false, use_prev_marking); + VerifyRegionClosure blk(allow_dirty, false, vo); heap_region_iterate(&blk); if (blk.failures()) { failures = true; @@ -2890,7 +2931,7 @@ void G1CollectedHeap::verify(bool allow_dirty, #ifndef PRODUCT if (VerifyDuringGC && G1VerifyDuringGCPrintReachable) { concurrent_mark()->print_reachable("at-verification-failure", - use_prev_marking, false /* all */); + vo, false /* all */); } #endif gclog_or_tty->flush(); @@ -3038,24 +3079,6 @@ G1CollectedHeap::doConcurrentMark() { } } -class VerifyMarkedObjsClosure: public ObjectClosure { - G1CollectedHeap* _g1h; - public: - VerifyMarkedObjsClosure(G1CollectedHeap* g1h) : _g1h(g1h) {} - void do_object(oop obj) { - assert(obj->mark()->is_marked() ? !_g1h->is_obj_dead(obj) : true, - "markandsweep mark should agree with concurrent deadness"); - } -}; - -void -G1CollectedHeap::checkConcurrentMark() { - VerifyMarkedObjsClosure verifycl(this); - // MutexLockerEx x(getMarkBitMapLock(), - // Mutex::_no_safepoint_check_flag); - object_iterate(&verifycl, false); -} - void G1CollectedHeap::do_sync_mark() { _cm->checkpointRootsInitial(); _cm->markFromRoots(); @@ -3252,7 +3275,10 @@ G1CollectedHeap::do_collection_pause_at_safepoint(double target_pause_time_ms) { HandleMark hm; // Discard invalid handles created during verification gclog_or_tty->print(" VerifyBeforeGC:"); prepare_for_verify(); - Universe::verify(false); + Universe::verify(/* allow dirty */ false, + /* silent */ false, + /* option */ VerifyOption_G1UsePrevMarking); + } COMPILER2_PRESENT(DerivedPointerTable::clear()); @@ -3424,7 +3450,9 @@ G1CollectedHeap::do_collection_pause_at_safepoint(double target_pause_time_ms) { HandleMark hm; // Discard invalid handles created during verification gclog_or_tty->print(" VerifyAfterGC:"); prepare_for_verify(); - Universe::verify(false); + Universe::verify(/* allow dirty */ true, + /* silent */ false, + /* option */ VerifyOption_G1UsePrevMarking); } if (was_enabled) ref_processor()->enable_discovery(); diff --git a/src/share/vm/gc_implementation/g1/g1CollectedHeap.hpp b/src/share/vm/gc_implementation/g1/g1CollectedHeap.hpp index c3e255ccb..0cc1d705b 100644 --- a/src/share/vm/gc_implementation/g1/g1CollectedHeap.hpp +++ b/src/share/vm/gc_implementation/g1/g1CollectedHeap.hpp @@ -1347,14 +1347,20 @@ public: // Perform verification. - // use_prev_marking == true -> use "prev" marking information, - // use_prev_marking == false -> use "next" marking information + // vo == UsePrevMarking -> use "prev" marking information, + // vo == UseNextMarking -> use "next" marking information + // vo == UseMarkWord -> use the mark word in the object header + // // NOTE: Only the "prev" marking information is guaranteed to be // consistent most of the time, so most calls to this should use - // use_prev_marking == true. Currently, there is only one case where - // this is called with use_prev_marking == false, which is to verify - // the "next" marking information at the end of remark. - void verify(bool allow_dirty, bool silent, bool use_prev_marking); + // vo == UsePrevMarking. + // Currently, there is only one case where this is called with + // vo == UseNextMarking, which is to verify the "next" marking + // information at the end of remark. + // Currently there is only one place where this is called with + // vo == UseMarkWord, which is to verify the marking during a + // full GC. + void verify(bool allow_dirty, bool silent, VerifyOption vo); // Override; it uses the "prev" marking information virtual void verify(bool allow_dirty, bool silent); @@ -1402,24 +1408,27 @@ public: // bitmap off to the side. void doConcurrentMark(); - // This is called from the marksweep collector which then does - // a concurrent mark and verifies that the results agree with - // the stop the world marking. - void checkConcurrentMark(); + // Do a full concurrent marking, synchronously. void do_sync_mark(); bool isMarkedPrev(oop obj) const; bool isMarkedNext(oop obj) const; - // use_prev_marking == true -> use "prev" marking information, - // use_prev_marking == false -> use "next" marking information + // vo == UsePrevMarking -> use "prev" marking information, + // vo == UseNextMarking -> use "next" marking information, + // vo == UseMarkWord -> use mark word from object header bool is_obj_dead_cond(const oop obj, const HeapRegion* hr, - const bool use_prev_marking) const { - if (use_prev_marking) { - return is_obj_dead(obj, hr); - } else { - return is_obj_ill(obj, hr); + const VerifyOption vo) const { + + switch (vo) { + case VerifyOption_G1UsePrevMarking: + return is_obj_dead(obj, hr); + case VerifyOption_G1UseNextMarking: + return is_obj_ill(obj, hr); + default: + assert(vo == VerifyOption_G1UseMarkWord, "must be"); + return !obj->is_gc_marked(); } } @@ -1460,18 +1469,24 @@ public: // Added if it is in permanent gen it isn't dead. // Added if it is NULL it isn't dead. - // use_prev_marking == true -> use "prev" marking information, - // use_prev_marking == false -> use "next" marking information + // vo == UsePrevMarking -> use "prev" marking information, + // vo == UseNextMarking -> use "next" marking information, + // vo == UseMarkWord -> use mark word from object header bool is_obj_dead_cond(const oop obj, - const bool use_prev_marking) { - if (use_prev_marking) { - return is_obj_dead(obj); - } else { - return is_obj_ill(obj); + const VerifyOption vo) const { + + switch (vo) { + case VerifyOption_G1UsePrevMarking: + return is_obj_dead(obj); + case VerifyOption_G1UseNextMarking: + return is_obj_ill(obj); + default: + assert(vo == VerifyOption_G1UseMarkWord, "must be"); + return !obj->is_gc_marked(); } } - bool is_obj_dead(const oop obj) { + bool is_obj_dead(const oop obj) const { const HeapRegion* hr = heap_region_containing(obj); if (hr == NULL) { if (Universe::heap()->is_in_permanent(obj)) @@ -1482,7 +1497,7 @@ public: else return is_obj_dead(obj, hr); } - bool is_obj_ill(const oop obj) { + bool is_obj_ill(const oop obj) const { const HeapRegion* hr = heap_region_containing(obj); if (hr == NULL) { if (Universe::heap()->is_in_permanent(obj)) diff --git a/src/share/vm/gc_implementation/g1/g1MarkSweep.cpp b/src/share/vm/gc_implementation/g1/g1MarkSweep.cpp index 4c93507a3..1ed4f993d 100644 --- a/src/share/vm/gc_implementation/g1/g1MarkSweep.cpp +++ b/src/share/vm/gc_implementation/g1/g1MarkSweep.cpp @@ -84,11 +84,6 @@ void G1MarkSweep::invoke_at_safepoint(ReferenceProcessor* rp, mark_sweep_phase1(marked_for_unloading, clear_all_softrefs); - if (VerifyDuringGC) { - G1CollectedHeap* g1h = G1CollectedHeap::heap(); - g1h->checkConcurrentMark(); - } - mark_sweep_phase2(); // Don't add any more derived pointers during phase3 @@ -179,6 +174,29 @@ void G1MarkSweep::mark_sweep_phase1(bool& marked_for_unloading, assert(GenMarkSweep::_marking_stack.is_empty(), "stack should be empty by now"); + + if (VerifyDuringGC) { + HandleMark hm; // handle scope + COMPILER2_PRESENT(DerivedPointerTableDeactivate dpt_deact); + gclog_or_tty->print(" VerifyDuringGC:(full)[Verifying "); + Universe::heap()->prepare_for_verify(); + // Note: we can verify only the heap here. When an object is + // marked, the previous value of the mark word (including + // identity hash values, ages, etc) is preserved, and the mark + // word is set to markOop::marked_value - effectively removing + // any hash values from the mark word. These hash values are + // used when verifying the dictionaries and so removing them + // from the mark word can make verification of the dictionaries + // fail. At the end of the GC, the orginal mark word values + // (including hash values) are restored to the appropriate + // objects. + Universe::heap()->verify(/* allow dirty */ true, + /* silent */ false, + /* option */ VerifyOption_G1UseMarkWord); + + G1CollectedHeap* g1h = G1CollectedHeap::heap(); + gclog_or_tty->print_cr("]"); + } } class G1PrepareCompactClosure: public HeapRegionClosure { diff --git a/src/share/vm/gc_implementation/g1/heapRegion.cpp b/src/share/vm/gc_implementation/g1/heapRegion.cpp index 6d2b27fd9..3acd1001c 100644 --- a/src/share/vm/gc_implementation/g1/heapRegion.cpp +++ b/src/share/vm/gc_implementation/g1/heapRegion.cpp @@ -60,13 +60,14 @@ private: oop _containing_obj; bool _failures; int _n_failures; - bool _use_prev_marking; + VerifyOption _vo; public: - // use_prev_marking == true -> use "prev" marking information, - // use_prev_marking == false -> use "next" marking information - VerifyLiveClosure(G1CollectedHeap* g1h, bool use_prev_marking) : + // _vo == UsePrevMarking -> use "prev" marking information, + // _vo == UseNextMarking -> use "next" marking information, + // _vo == UseMarkWord -> use mark word from object header. + VerifyLiveClosure(G1CollectedHeap* g1h, VerifyOption vo) : _g1h(g1h), _bs(NULL), _containing_obj(NULL), - _failures(false), _n_failures(0), _use_prev_marking(use_prev_marking) + _failures(false), _n_failures(0), _vo(vo) { BarrierSet* bs = _g1h->barrier_set(); if (bs->is_a(BarrierSet::CardTableModRef)) @@ -95,14 +96,14 @@ public: template void do_oop_work(T* p) { assert(_containing_obj != NULL, "Precondition"); - assert(!_g1h->is_obj_dead_cond(_containing_obj, _use_prev_marking), + assert(!_g1h->is_obj_dead_cond(_containing_obj, _vo), "Precondition"); T heap_oop = oopDesc::load_heap_oop(p); if (!oopDesc::is_null(heap_oop)) { oop obj = oopDesc::decode_heap_oop_not_null(heap_oop); bool failed = false; if (!_g1h->is_in_closed_subset(obj) || - _g1h->is_obj_dead_cond(obj, _use_prev_marking)) { + _g1h->is_obj_dead_cond(obj, _vo)) { if (!_failures) { gclog_or_tty->print_cr(""); gclog_or_tty->print_cr("----------"); @@ -735,20 +736,20 @@ void HeapRegion::print_on(outputStream* st) const { void HeapRegion::verify(bool allow_dirty) const { bool dummy = false; - verify(allow_dirty, /* use_prev_marking */ true, /* failures */ &dummy); + verify(allow_dirty, VerifyOption_G1UsePrevMarking, /* failures */ &dummy); } // This really ought to be commoned up into OffsetTableContigSpace somehow. // We would need a mechanism to make that code skip dead objects. void HeapRegion::verify(bool allow_dirty, - bool use_prev_marking, + VerifyOption vo, bool* failures) const { G1CollectedHeap* g1 = G1CollectedHeap::heap(); *failures = false; HeapWord* p = bottom(); HeapWord* prev_p = NULL; - VerifyLiveClosure vl_cl(g1, use_prev_marking); + VerifyLiveClosure vl_cl(g1, vo); bool is_humongous = isHumongous(); bool do_bot_verify = !is_young(); size_t object_num = 0; @@ -773,7 +774,7 @@ void HeapRegion::verify(bool allow_dirty, return; } - if (!g1->is_obj_dead_cond(obj, this, use_prev_marking)) { + if (!g1->is_obj_dead_cond(obj, this, vo)) { if (obj->is_oop()) { klassOop klass = obj->klass(); if (!klass->is_perm()) { diff --git a/src/share/vm/gc_implementation/g1/heapRegion.hpp b/src/share/vm/gc_implementation/g1/heapRegion.hpp index 11547cd75..ec63d1fa9 100644 --- a/src/share/vm/gc_implementation/g1/heapRegion.hpp +++ b/src/share/vm/gc_implementation/g1/heapRegion.hpp @@ -855,14 +855,20 @@ class HeapRegion: public G1OffsetTableContigSpace { void print() const; void print_on(outputStream* st) const; - // use_prev_marking == true -> use "prev" marking information, - // use_prev_marking == false -> use "next" marking information + // vo == UsePrevMarking -> use "prev" marking information, + // vo == UseNextMarking -> use "next" marking information + // vo == UseMarkWord -> use the mark word in the object header + // // NOTE: Only the "prev" marking information is guaranteed to be // consistent most of the time, so most calls to this should use - // use_prev_marking == true. Currently, there is only one case where - // this is called with use_prev_marking == false, which is to verify - // the "next" marking information at the end of remark. - void verify(bool allow_dirty, bool use_prev_marking, bool *failures) const; + // vo == UsePrevMarking. + // Currently, there is only one case where this is called with + // vo == UseNextMarking, which is to verify the "next" marking + // information at the end of remark. + // Currently there is only one place where this is called with + // vo == UseMarkWord, which is to verify the marking during a + // full GC. + void verify(bool allow_dirty, VerifyOption vo, bool *failures) const; // Override; it uses the "prev" marking information virtual void verify(bool allow_dirty) const; diff --git a/src/share/vm/gc_implementation/parallelScavenge/parallelScavengeHeap.cpp b/src/share/vm/gc_implementation/parallelScavenge/parallelScavengeHeap.cpp index d06b62d63..6d2ac49bc 100644 --- a/src/share/vm/gc_implementation/parallelScavenge/parallelScavengeHeap.cpp +++ b/src/share/vm/gc_implementation/parallelScavenge/parallelScavengeHeap.cpp @@ -901,7 +901,7 @@ void ParallelScavengeHeap::print_tracing_info() const { } -void ParallelScavengeHeap::verify(bool allow_dirty, bool silent, bool option /* ignored */) { +void ParallelScavengeHeap::verify(bool allow_dirty, bool silent, VerifyOption option /* ignored */) { // Why do we need the total_collections()-filter below? if (total_collections() > 0) { if (!silent) { diff --git a/src/share/vm/gc_implementation/parallelScavenge/parallelScavengeHeap.hpp b/src/share/vm/gc_implementation/parallelScavenge/parallelScavengeHeap.hpp index 09e0124ed..e23b8eec9 100644 --- a/src/share/vm/gc_implementation/parallelScavenge/parallelScavengeHeap.hpp +++ b/src/share/vm/gc_implementation/parallelScavenge/parallelScavengeHeap.hpp @@ -253,7 +253,7 @@ CollectorPolicy* collector_policy() const { return (CollectorPolicy*) _collector virtual void gc_threads_do(ThreadClosure* tc) const; virtual void print_tracing_info() const; - void verify(bool allow_dirty, bool silent, bool /* option */); + void verify(bool allow_dirty, bool silent, VerifyOption option /* ignored */); void print_heap_change(size_t prev_used); diff --git a/src/share/vm/gc_interface/collectedHeap.hpp b/src/share/vm/gc_interface/collectedHeap.hpp index f5082e4d2..7c26a5272 100644 --- a/src/share/vm/gc_interface/collectedHeap.hpp +++ b/src/share/vm/gc_interface/collectedHeap.hpp @@ -606,7 +606,7 @@ class CollectedHeap : public CHeapObj { virtual void print_tracing_info() const = 0; // Heap verification - virtual void verify(bool allow_dirty, bool silent, bool option) = 0; + virtual void verify(bool allow_dirty, bool silent, VerifyOption option) = 0; // Non product verification and debugging. #ifndef PRODUCT diff --git a/src/share/vm/memory/genCollectedHeap.cpp b/src/share/vm/memory/genCollectedHeap.cpp index 940a46c10..54cb93d9c 100644 --- a/src/share/vm/memory/genCollectedHeap.cpp +++ b/src/share/vm/memory/genCollectedHeap.cpp @@ -1260,7 +1260,7 @@ GCStats* GenCollectedHeap::gc_stats(int level) const { return _gens[level]->gc_stats(); } -void GenCollectedHeap::verify(bool allow_dirty, bool silent, bool option /* ignored */) { +void GenCollectedHeap::verify(bool allow_dirty, bool silent, VerifyOption option /* ignored */) { if (!silent) { gclog_or_tty->print("permgen "); } diff --git a/src/share/vm/memory/genCollectedHeap.hpp b/src/share/vm/memory/genCollectedHeap.hpp index 45f72bae1..c060da4c3 100644 --- a/src/share/vm/memory/genCollectedHeap.hpp +++ b/src/share/vm/memory/genCollectedHeap.hpp @@ -361,7 +361,7 @@ public: void prepare_for_verify(); // Override. - void verify(bool allow_dirty, bool silent, bool /* option */); + void verify(bool allow_dirty, bool silent, VerifyOption option); // Override. void print() const; diff --git a/src/share/vm/memory/universe.cpp b/src/share/vm/memory/universe.cpp index 9499d6bac..ff14097e4 100644 --- a/src/share/vm/memory/universe.cpp +++ b/src/share/vm/memory/universe.cpp @@ -1278,7 +1278,7 @@ void Universe::print_heap_after_gc(outputStream* st) { st->print_cr("}"); } -void Universe::verify(bool allow_dirty, bool silent, bool option) { +void Universe::verify(bool allow_dirty, bool silent, VerifyOption option) { if (SharedSkipVerify) { return; } diff --git a/src/share/vm/memory/universe.hpp b/src/share/vm/memory/universe.hpp index 86682682f..04f289af0 100644 --- a/src/share/vm/memory/universe.hpp +++ b/src/share/vm/memory/universe.hpp @@ -109,6 +109,14 @@ struct NarrowOopStruct { bool _use_implicit_null_checks; }; +enum VerifyOption { + VerifyOption_Default = 0, + + // G1 + VerifyOption_G1UsePrevMarking = VerifyOption_Default, + VerifyOption_G1UseNextMarking = VerifyOption_G1UsePrevMarking + 1, + VerifyOption_G1UseMarkWord = VerifyOption_G1UseNextMarking + 1 +}; class Universe: AllStatic { // Ugh. Universe is much too friendly. @@ -404,7 +412,8 @@ class Universe: AllStatic { // Debugging static bool verify_in_progress() { return _verify_in_progress; } - static void verify(bool allow_dirty = true, bool silent = false, bool option = true); + static void verify(bool allow_dirty = true, bool silent = false, + VerifyOption option = VerifyOption_Default ); static int verify_count() { return _verify_count; } static void print(); static void print_on(outputStream* st); -- GitLab