From afc4bd682f312267c68ec9d49c4464f1659f1406 Mon Sep 17 00:00:00 2001 From: iveresov Date: Thu, 3 Jul 2008 03:17:29 -0700 Subject: [PATCH] 6702387: G1: assertion failure: assert(p == current_top || oop(p)->is_oop(),"p is not a block start") Summary: Do not coalesce dead and moved objects when removing self-forwarding pointers during the evacuation failure. Also fixed a issue in a BOT refinement code for TLABs. Reviewed-by: tonyp, jcoomes --- .../g1/g1BlockOffsetTable.cpp | 6 +- .../gc_implementation/g1/g1CollectedHeap.cpp | 90 +++++++++---------- 2 files changed, 45 insertions(+), 51 deletions(-) diff --git a/src/share/vm/gc_implementation/g1/g1BlockOffsetTable.cpp b/src/share/vm/gc_implementation/g1/g1BlockOffsetTable.cpp index c125fbe5d..4d9d7f0ce 100644 --- a/src/share/vm/gc_implementation/g1/g1BlockOffsetTable.cpp +++ b/src/share/vm/gc_implementation/g1/g1BlockOffsetTable.cpp @@ -412,7 +412,11 @@ G1BlockOffsetArray::forward_to_block_containing_addr_slow(HeapWord* q, // offset table was actually a lab allocation, and was divided into // several objects subsequently. Fix this situation as we answer the // query, by updating entries as we cross them. - size_t next_index = _array->index_for(n) + 1; + + // If the fist object's end q is at the card boundary. Start refining + // with the corresponding card (the value of the entry will be basically + // set to 0). If the object crosses the boundary -- start from the next card. + size_t next_index = _array->index_for(n) + !_array->is_card_boundary(n); HeapWord* next_boundary = _array->address_for_index(next_index); if (csp() != NULL) { if (addr >= csp()->top()) return csp()->top(); diff --git a/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp b/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp index a4d96e023..82db0c37e 100644 --- a/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp +++ b/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp @@ -2750,68 +2750,59 @@ private: G1CollectedHeap* _g1; ConcurrentMark* _cm; HeapRegion* _hr; - HeapWord* _last_self_forwarded_end; size_t _prev_marked_bytes; size_t _next_marked_bytes; public: RemoveSelfPointerClosure(G1CollectedHeap* g1, HeapRegion* hr) : _g1(g1), _cm(_g1->concurrent_mark()), _hr(hr), - _last_self_forwarded_end(_hr->bottom()), _prev_marked_bytes(0), _next_marked_bytes(0) {} size_t prev_marked_bytes() { return _prev_marked_bytes; } size_t next_marked_bytes() { return _next_marked_bytes; } - void fill_remainder() { - HeapWord* limit = _hr->top(); - MemRegion mr(_last_self_forwarded_end, limit); - if (!mr.is_empty()) { - SharedHeap::fill_region_with_object(mr); - _cm->clearRangeBothMaps(mr); - _hr->declare_filled_region_to_BOT(mr); - } - } - + // The original idea here was to coalesce evacuated and dead objects. + // However that caused complications with the block offset table (BOT). + // In particular if there were two TLABs, one of them partially refined. + // |----- TLAB_1--------|----TLAB_2-~~~(partially refined part)~~~| + // The BOT entries of the unrefined part of TLAB_2 point to the start + // of TLAB_2. If the last object of the TLAB_1 and the first object + // of TLAB_2 are coalesced, then the cards of the unrefined part + // would point into middle of the filler object. + // + // The current approach is to not coalesce and leave the BOT contents intact. void do_object(oop obj) { - if (obj->is_forwarded()) { - if (obj->forwardee() == obj) { - assert(!_g1->is_obj_dead(obj), "We should not be preserving dead objs."); - _cm->markPrev(obj); - assert(_cm->isPrevMarked(obj), "Should be marked!"); - _prev_marked_bytes += (obj->size() * HeapWordSize); - if (_g1->mark_in_progress() && !_g1->is_obj_ill(obj)) { - _cm->markAndGrayObjectIfNecessary(obj); - } - HeapWord* obj_start = (HeapWord*)obj; - if (obj_start > _last_self_forwarded_end) { - MemRegion mr(_last_self_forwarded_end, obj_start); - SharedHeap::fill_region_with_object(mr); - assert(_cm->isPrevMarked(obj), "Should be marked!"); - _cm->clearRangeBothMaps(mr); - assert(_cm->isPrevMarked(obj), "Should be marked!"); - _hr->declare_filled_region_to_BOT(mr); - } - _last_self_forwarded_end = obj_start + obj->size(); - obj->set_mark(markOopDesc::prototype()); - - // While we were processing RSet buffers during the - // collection, we actually didn't scan any cards on the - // collection set, since we didn't want to update remebered - // sets with entries that point into the collection set, given - // that live objects fromthe collection set are about to move - // and such entries will be stale very soon. This change also - // dealt with a reliability issue which involved scanning a - // card in the collection set and coming across an array that - // was being chunked and looking malformed. The problem is - // that, if evacuation fails, we might have remembered set - // entries missing given that we skipped cards on the - // collection set. So, we'll recreate such entries now. - RecreateRSetEntriesClosure cl(_g1, _hr); - obj->oop_iterate(&cl); - - assert(_cm->isPrevMarked(obj), "Should be marked!"); + if (obj->is_forwarded() && obj->forwardee() == obj) { + // The object failed to move. + assert(!_g1->is_obj_dead(obj), "We should not be preserving dead objs."); + _cm->markPrev(obj); + assert(_cm->isPrevMarked(obj), "Should be marked!"); + _prev_marked_bytes += (obj->size() * HeapWordSize); + if (_g1->mark_in_progress() && !_g1->is_obj_ill(obj)) { + _cm->markAndGrayObjectIfNecessary(obj); } + obj->set_mark(markOopDesc::prototype()); + // While we were processing RSet buffers during the + // collection, we actually didn't scan any cards on the + // collection set, since we didn't want to update remebered + // sets with entries that point into the collection set, given + // that live objects fromthe collection set are about to move + // and such entries will be stale very soon. This change also + // dealt with a reliability issue which involved scanning a + // card in the collection set and coming across an array that + // was being chunked and looking malformed. The problem is + // that, if evacuation fails, we might have remembered set + // entries missing given that we skipped cards on the + // collection set. So, we'll recreate such entries now. + RecreateRSetEntriesClosure cl(_g1, _hr); + obj->oop_iterate(&cl); + assert(_cm->isPrevMarked(obj), "Should be marked!"); + } else { + // The object has been either evacuated or is dead. Fill it with a + // dummy object. + MemRegion mr((HeapWord*)obj, obj->size()); + SharedHeap::fill_region_with_object(mr); + _cm->clearRangeBothMaps(mr); } } }; @@ -2826,7 +2817,6 @@ void G1CollectedHeap::remove_self_forwarding_pointers() { RemoveSelfPointerClosure rspc(_g1h, cur); assert(cur->in_collection_set(), "bad CS"); cur->object_iterate(&rspc); - rspc.fill_remainder(); // A number of manipulations to make the TAMS be the current top, // and the marked bytes be the ones observed in the iteration. -- GitLab