6882730: G1: parallel heap verification messes up region dump
Summary: It tidies up the G1 heap verification a bit. In particular, when the verification is done in parallel and there is a failure, this is propagated to the top level and the heap is dumped at the end, not by every thread that encounters a failure.
Reviewed-by: johnc, jmasa
--- a/hotspot/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp Fri Oct 02 16:12:07 2009 -0400
+++ b/hotspot/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp Fri Oct 02 16:20:42 2009 -0400
@@ -2210,40 +2210,58 @@
bool _allow_dirty;
bool _par;
bool _use_prev_marking;
+ 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)
: _allow_dirty(allow_dirty),
_par(par),
- _use_prev_marking(use_prev_marking) {}
+ _use_prev_marking(use_prev_marking),
+ _failures(false) {}
+
+ bool failures() {
+ return _failures;
+ }
bool doHeapRegion(HeapRegion* r) {
guarantee(_par || r->claim_value() == HeapRegion::InitialClaimValue,
"Should be unclaimed at verify points.");
if (!r->continuesHumongous()) {
- VerifyObjsInRegionClosure not_dead_yet_cl(r, _use_prev_marking);
- r->verify(_allow_dirty, _use_prev_marking);
- r->object_iterate(¬_dead_yet_cl);
- guarantee(r->max_live_bytes() >= not_dead_yet_cl.live_bytes(),
- "More live objects than counted in last complete marking.");
+ bool failures = false;
+ r->verify(_allow_dirty, _use_prev_marking, &failures);
+ if (failures) {
+ _failures = true;
+ } else {
+ VerifyObjsInRegionClosure not_dead_yet_cl(r, _use_prev_marking);
+ 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"] "
+ "max_live_bytes "SIZE_FORMAT" "
+ "< calculated "SIZE_FORMAT,
+ r->bottom(), r->end(),
+ r->max_live_bytes(),
+ not_dead_yet_cl.live_bytes());
+ _failures = true;
+ }
+ }
}
- return false;
+ return false; // stop the region iteration if we hit a failure
}
};
class VerifyRootsClosure: public OopsInGenClosure {
private:
G1CollectedHeap* _g1h;
+ bool _use_prev_marking;
bool _failures;
- bool _use_prev_marking;
public:
// use_prev_marking == true -> use "prev" marking information,
// use_prev_marking == false -> use "next" marking information
VerifyRootsClosure(bool use_prev_marking) :
_g1h(G1CollectedHeap::heap()),
- _failures(false),
- _use_prev_marking(use_prev_marking) { }
+ _use_prev_marking(use_prev_marking),
+ _failures(false) { }
bool failures() { return _failures; }
@@ -2253,7 +2271,7 @@
oop obj = oopDesc::decode_heap_oop_not_null(heap_oop);
if (_g1h->is_obj_dead_cond(obj, _use_prev_marking)) {
gclog_or_tty->print_cr("Root location "PTR_FORMAT" "
- "points to dead obj "PTR_FORMAT, p, (void*) obj);
+ "points to dead obj "PTR_FORMAT, p, (void*) obj);
obj->print_on(gclog_or_tty);
_failures = true;
}
@@ -2271,6 +2289,7 @@
G1CollectedHeap* _g1h;
bool _allow_dirty;
bool _use_prev_marking;
+ bool _failures;
public:
// use_prev_marking == true -> use "prev" marking information,
@@ -2280,13 +2299,21 @@
AbstractGangTask("Parallel verify task"),
_g1h(g1h),
_allow_dirty(allow_dirty),
- _use_prev_marking(use_prev_marking) { }
+ _use_prev_marking(use_prev_marking),
+ _failures(false) { }
+
+ bool failures() {
+ return _failures;
+ }
void work(int worker_i) {
HandleMark hm;
VerifyRegionClosure blk(_allow_dirty, true, _use_prev_marking);
_g1h->heap_region_par_iterate_chunked(&blk, worker_i,
HeapRegion::ParVerifyClaimValue);
+ if (blk.failures()) {
+ _failures = true;
+ }
}
};
@@ -2307,6 +2334,7 @@
&rootsCl,
&blobsCl,
&rootsCl);
+ bool failures = rootsCl.failures();
rem_set()->invalidate(perm_gen()->used_region(), false);
if (!silent) { gclog_or_tty->print("heapRegions "); }
if (GCParallelVerificationEnabled && ParallelGCThreads > 1) {
@@ -2318,6 +2346,9 @@
set_par_threads(n_workers);
workers()->run_task(&task);
set_par_threads(0);
+ if (task.failures()) {
+ failures = true;
+ }
assert(check_heap_region_claim_values(HeapRegion::ParVerifyClaimValue),
"sanity check");
@@ -2329,10 +2360,23 @@
} else {
VerifyRegionClosure blk(allow_dirty, false, use_prev_marking);
_hrs->iterate(&blk);
+ if (blk.failures()) {
+ failures = true;
+ }
}
if (!silent) gclog_or_tty->print("remset ");
rem_set()->verify();
- guarantee(!rootsCl.failures(), "should not have had failures");
+
+ if (failures) {
+ gclog_or_tty->print_cr("Heap:");
+ print_on(gclog_or_tty, true /* extended */);
+ gclog_or_tty->print_cr("");
+ if (VerifyDuringGC && G1VerifyConcMarkPrintReachable) {
+ concurrent_mark()->print_prev_bitmap_reachable();
+ }
+ gclog_or_tty->flush();
+ }
+ guarantee(!failures, "there should not have been any failures");
} else {
if (!silent) gclog_or_tty->print("(SKIPPING roots, heapRegions, remset) ");
}
@@ -2374,6 +2418,7 @@
st->cr();
perm()->as_gen()->print_on(st);
if (extended) {
+ st->cr();
print_on_extended(st);
}
}
--- a/hotspot/src/share/vm/gc_implementation/g1/heapRegion.cpp Fri Oct 02 16:12:07 2009 -0400
+++ b/hotspot/src/share/vm/gc_implementation/g1/heapRegion.cpp Fri Oct 02 16:20:42 2009 -0400
@@ -722,12 +722,13 @@
st->print(" F");
else
st->print(" ");
- st->print(" %d", _gc_time_stamp);
+ st->print(" %5d", _gc_time_stamp);
G1OffsetTableContigSpace::print_on(st);
}
void HeapRegion::verify(bool allow_dirty) const {
- verify(allow_dirty, /* use_prev_marking */ true);
+ bool dummy = false;
+ verify(allow_dirty, /* use_prev_marking */ true, /* failures */ &dummy);
}
#define OBJ_SAMPLE_INTERVAL 0
@@ -736,8 +737,11 @@
// 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) const {
+void HeapRegion::verify(bool allow_dirty,
+ bool use_prev_marking,
+ bool* failures) const {
G1CollectedHeap* g1 = G1CollectedHeap::heap();
+ *failures = false;
HeapWord* p = bottom();
HeapWord* prev_p = NULL;
int objs = 0;
@@ -746,8 +750,14 @@
while (p < top()) {
size_t size = oop(p)->size();
if (blocks == BLOCK_SAMPLE_INTERVAL) {
- guarantee(p == block_start_const(p + (size/2)),
- "check offset computation");
+ HeapWord* res = block_start_const(p + (size/2));
+ if (p != res) {
+ gclog_or_tty->print_cr("offset computation 1 for "PTR_FORMAT" and "
+ SIZE_FORMAT" returned "PTR_FORMAT,
+ p, size, res);
+ *failures = true;
+ return;
+ }
blocks = 0;
} else {
blocks++;
@@ -755,11 +765,34 @@
if (objs == OBJ_SAMPLE_INTERVAL) {
oop obj = oop(p);
if (!g1->is_obj_dead_cond(obj, this, use_prev_marking)) {
- obj->verify();
- vl_cl.set_containing_obj(obj);
- obj->oop_iterate(&vl_cl);
- if (G1MaxVerifyFailures >= 0
- && vl_cl.n_failures() >= G1MaxVerifyFailures) break;
+ if (obj->is_oop()) {
+ klassOop klass = obj->klass();
+ if (!klass->is_perm()) {
+ gclog_or_tty->print_cr("klass "PTR_FORMAT" of object "PTR_FORMAT" "
+ "not in perm", klass, obj);
+ *failures = true;
+ return;
+ } else if (!klass->is_klass()) {
+ gclog_or_tty->print_cr("klass "PTR_FORMAT" of object "PTR_FORMAT" "
+ "not a klass", klass, obj);
+ *failures = true;
+ return;
+ } else {
+ vl_cl.set_containing_obj(obj);
+ obj->oop_iterate(&vl_cl);
+ if (vl_cl.failures()) {
+ *failures = true;
+ }
+ if (G1MaxVerifyFailures >= 0 &&
+ vl_cl.n_failures() >= G1MaxVerifyFailures) {
+ return;
+ }
+ }
+ } else {
+ gclog_or_tty->print_cr(PTR_FORMAT" no an oop", obj);
+ *failures = true;
+ return;
+ }
}
objs = 0;
} else {
@@ -771,21 +804,22 @@
HeapWord* rend = end();
HeapWord* rtop = top();
if (rtop < rend) {
- guarantee(block_start_const(rtop + (rend - rtop) / 2) == rtop,
- "check offset computation");
- }
- if (vl_cl.failures()) {
- gclog_or_tty->print_cr("Heap:");
- G1CollectedHeap::heap()->print_on(gclog_or_tty, true /* extended */);
- gclog_or_tty->print_cr("");
+ HeapWord* res = block_start_const(rtop + (rend - rtop) / 2);
+ if (res != rtop) {
+ gclog_or_tty->print_cr("offset computation 2 for "PTR_FORMAT" and "
+ PTR_FORMAT" returned "PTR_FORMAT,
+ rtop, rend, res);
+ *failures = true;
+ return;
+ }
}
- if (VerifyDuringGC &&
- G1VerifyConcMarkPrintReachable &&
- vl_cl.failures()) {
- g1->concurrent_mark()->print_prev_bitmap_reachable();
+
+ if (p != top()) {
+ gclog_or_tty->print_cr("end of last object "PTR_FORMAT" "
+ "does not match top "PTR_FORMAT, p, top());
+ *failures = true;
+ return;
}
- guarantee(!vl_cl.failures(), "region verification failed");
- guarantee(p == top(), "end of last object must match end of space");
}
// G1OffsetTableContigSpace code; copied from space.cpp. Hope this can go
--- a/hotspot/src/share/vm/gc_implementation/g1/heapRegion.hpp Fri Oct 02 16:12:07 2009 -0400
+++ b/hotspot/src/share/vm/gc_implementation/g1/heapRegion.hpp Fri Oct 02 16:20:42 2009 -0400
@@ -798,7 +798,7 @@
// 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) const;
+ void verify(bool allow_dirty, bool use_prev_marking, bool *failures) const;
// Override; it uses the "prev" marking information
virtual void verify(bool allow_dirty) const;