stan/jdk-24
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

6978187: G1: assert(ParallelGCThreads> 1 || n_yielded() == _hrrs->occupied()) strikes again

Browse Source 
An evacuation failure while copying the roots caused an object, A, to be forwarded to itself. During the subsequent RSet updating a reference to A was processed causing the reference to be added to the RSet of A's heap region. As a result of adding to the remembered set we ran into the issue described in 6930581 - the sparse table expanded and the RSet scanning code walked the cards in one instance of RHashTable (_cur) while the occupied() counts the cards in the expanded table (_next).

Reviewed-by: tonyp, iveresov
This commit is contained in:
John Cuthbertson 2010-11-16 14:07:33 -08:00
parent fbe476856f
commit e5344b854c
4 changed files with 122 additions and 84 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
hotspot/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp
View File

@ -795,6 +795,7 @@ public:
_worker_i(worker_i),
_g1h(g1)
{ }
bool doHeapRegion(HeapRegion* r) {
if (!r->continuesHumongous()) {
_cl.set_from(r);

28
hotspot/src/share/vm/gc_implementation/g1/g1RemSet.cpp
View File

@ -116,7 +116,6 @@ G1RemSet::G1RemSet(G1CollectedHeap* g1, CardTableModRefBS* ct_bs)
: _g1(g1), _conc_refine_cards(0),
_ct_bs(ct_bs), _g1p(_g1->g1_policy()),
_cg1r(g1->concurrent_g1_refine()),
_traversal_in_progress(false),
_cset_rs_update_cl(NULL),
_cards_scanned(NULL), _total_cards_scanned(0)
{
@ -512,8 +511,6 @@ void G1RemSet::prepare_for_oops_into_collection_set_do() {
DirtyCardQueueSet& dcqs = JavaThread::dirty_card_queue_set();
dcqs.concatenate_logs();
assert(!_traversal_in_progress, "Invariant between iterations.");
set_traversal(true);
if (ParallelGCThreads > 0) {
_seq_task->set_n_threads((int)n_workers());
}
@ -539,9 +536,6 @@ class cleanUpIteratorsClosure : public HeapRegionClosure {
// through the oops which coincide with that card. It scans the reference
// fields in each oop; when it finds an oop that points into the collection
// set, the RSet for the region containing the referenced object is updated.
// Note: _par_traversal_in_progress in the G1RemSet must be FALSE; otherwise
// the UpdateRSetImmediate closure will cause cards to be enqueued on to
// the DCQS that we're iterating over, causing an infinite loop.
class UpdateRSetCardTableEntryIntoCSetClosure: public CardTableEntryClosure {
G1CollectedHeap* _g1;
CardTableModRefBS* _ct_bs;
@ -611,8 +605,6 @@ void G1RemSet::cleanup_after_oops_into_collection_set_do() {
// Set all cards back to clean.
_g1->cleanUpCardTable();
set_traversal(false);
DirtyCardQueueSet& into_cset_dcqs = _g1->into_cset_dirty_card_queue_set();
int into_cset_n_buffers = into_cset_dcqs.completed_buffers_num();
@ -645,21 +637,8 @@ void G1RemSet::cleanup_after_oops_into_collection_set_do() {
assert(_g1->into_cset_dirty_card_queue_set().completed_buffers_num() == 0,
"all buffers should be freed");
_g1->into_cset_dirty_card_queue_set().clear_n_completed_buffers();
assert(!_traversal_in_progress, "Invariant between iterations.");
}
class UpdateRSObjectClosure: public ObjectClosure {
UpdateRSOopClosure* _update_rs_oop_cl;
public:
UpdateRSObjectClosure(UpdateRSOopClosure* update_rs_oop_cl) :
_update_rs_oop_cl(update_rs_oop_cl) {}
void do_object(oop obj) {
obj->oop_iterate(_update_rs_oop_cl);
}
};
class ScrubRSClosure: public HeapRegionClosure {
G1CollectedHeap* _g1h;
BitMap* _region_bm;
@ -749,7 +728,12 @@ bool G1RemSet::concurrentRefineOneCard_impl(jbyte* card_ptr, int worker_i,
ct_freq_note_card(_ct_bs->index_for(start));
#endif
UpdateRSOopClosure update_rs_oop_cl(this, worker_i);
assert(!check_for_refs_into_cset || _cset_rs_update_cl[worker_i] != NULL, "sanity");
UpdateRSOrPushRefOopClosure update_rs_oop_cl(_g1,
_g1->g1_rem_set(),
_cset_rs_update_cl[worker_i],
check_for_refs_into_cset,
worker_i);
update_rs_oop_cl.set_from(r);
TriggerClosure trigger_cl;

72
hotspot/src/share/vm/gc_implementation/g1/g1RemSet.hpp
View File

@ -59,11 +59,6 @@ protected:
size_t* _cards_scanned;
size_t _total_cards_scanned;
// _traversal_in_progress is "true" iff a traversal is in progress.
bool _traversal_in_progress;
void set_traversal(bool b) { _traversal_in_progress = b; }
// Used for caching the closure that is responsible for scanning
// references into the collection set.
OopsInHeapRegionClosure** _cset_rs_update_cl;
@ -76,10 +71,6 @@ protected:
bool concurrentRefineOneCard_impl(jbyte* card_ptr, int worker_i,
bool check_for_refs_into_cset);
protected:
template <class T> void write_ref_nv(HeapRegion* from, T* p);
template <class T> void par_write_ref_nv(HeapRegion* from, T* p, int tid);
public:
// This is called to reset dual hash tables after the gc pause
// is finished and the initial hash table is no longer being
@ -117,22 +108,8 @@ public:
// Record, if necessary, the fact that *p (where "p" is in region "from",
// which is required to be non-NULL) has changed to a new non-NULL value.
// [Below the virtual version calls a non-virtual protected
// workhorse that is templatified for narrow vs wide oop.]
inline void write_ref(HeapRegion* from, oop* p) {
write_ref_nv(from, p);
}
inline void write_ref(HeapRegion* from, narrowOop* p) {
write_ref_nv(from, p);
}
inline void par_write_ref(HeapRegion* from, oop* p, int tid) {
par_write_ref_nv(from, p, tid);
}
inline void par_write_ref(HeapRegion* from, narrowOop* p, int tid) {
par_write_ref_nv(from, p, tid);
}
bool self_forwarded(oop obj);
template <class T> void write_ref(HeapRegion* from, T* p);
template <class T> void par_write_ref(HeapRegion* from, T* p, int tid);
// Requires "region_bm" and "card_bm" to be bitmaps with 1 bit per region
// or card, respectively, such that a region or card with a corresponding
@ -186,9 +163,8 @@ class UpdateRSOopClosure: public OopClosure {
public:
UpdateRSOopClosure(G1RemSet* rs, int worker_i = 0) :
_from(NULL), _rs(rs), _worker_i(worker_i) {
guarantee(_rs != NULL, "Requires an HRIntoG1RemSet");
}
_from(NULL), _rs(rs), _worker_i(worker_i)
{}
void set_from(HeapRegion* from) {
assert(from != NULL, "from region must be non-NULL");
@ -215,3 +191,43 @@ public:
virtual void do_oop(narrowOop* p) { do_oop_work(p); }
virtual void do_oop( oop* p) { do_oop_work(p); }
};
class UpdateRSOrPushRefOopClosure: public OopClosure {
G1CollectedHeap* _g1;
G1RemSet* _g1_rem_set;
HeapRegion* _from;
OopsInHeapRegionClosure* _push_ref_cl;
bool _record_refs_into_cset;
int _worker_i;
template <class T> void do_oop_work(T* p);
public:
UpdateRSOrPushRefOopClosure(G1CollectedHeap* g1h,
G1RemSet* rs,
OopsInHeapRegionClosure* push_ref_cl,
bool record_refs_into_cset,
int worker_i = 0) :
_g1(g1h),
_g1_rem_set(rs),
_from(NULL),
_record_refs_into_cset(record_refs_into_cset),
_push_ref_cl(push_ref_cl),
_worker_i(worker_i) { }
void set_from(HeapRegion* from) {
assert(from != NULL, "from region must be non-NULL");
_from = from;
}
bool self_forwarded(oop obj) {
bool result = (obj->is_forwarded() && (obj->forwardee()== obj));
return result;
}
virtual void do_oop(narrowOop* p) { do_oop_work(p); }
virtual void do_oop(oop* p) { do_oop_work(p); }
bool apply_to_weak_ref_discovered_field() { return true; }
};

105
hotspot/src/share/vm/gc_implementation/g1/g1RemSet.inline.hpp
View File

@ -31,17 +31,12 @@ inline size_t G1RemSet::n_workers() {
}
template <class T>
inline void G1RemSet::write_ref_nv(HeapRegion* from, T* p) {
par_write_ref_nv(from, p, 0);
}
inline bool G1RemSet::self_forwarded(oop obj) {
bool result = (obj->is_forwarded() && (obj->forwardee()== obj));
return result;
inline void G1RemSet::write_ref(HeapRegion* from, T* p) {
par_write_ref(from, p, 0);
}
template <class T>
inline void G1RemSet::par_write_ref_nv(HeapRegion* from, T* p, int tid) {
inline void G1RemSet::par_write_ref(HeapRegion* from, T* p, int tid) {
oop obj = oopDesc::load_decode_heap_oop(p);
#ifdef ASSERT
// can't do because of races
@ -62,34 +57,15 @@ inline void G1RemSet::par_write_ref_nv(HeapRegion* from, T* p, int tid) {
assert(from == NULL || from->is_in_reserved(p), "p is not in from");
HeapRegion* to = _g1->heap_region_containing(obj);
// The test below could be optimized by applying a bit op to to and from.
if (to != NULL && from != NULL && from != to) {
// The _traversal_in_progress flag is true during the collection pause,
// false during the evacuation failure handling. This should avoid a
// potential loop if we were to add the card containing 'p' to the DCQS
// that's used to regenerate the remembered sets for the collection set,
// in the event of an evacuation failure, here. The UpdateRSImmediate
// closure will eventally call this routine.
if (_traversal_in_progress &&
to->in_collection_set() && !self_forwarded(obj)) {
assert(_cset_rs_update_cl[tid] != NULL, "should have been set already");
_cset_rs_update_cl[tid]->do_oop(p);
// Deferred updates to the CSet are either discarded (in the normal case),
// or processed (if an evacuation failure occurs) at the end
// of the collection.
// See G1RemSet::cleanup_after_oops_into_collection_set_do().
} else {
if (to != NULL && from != to) {
#if G1_REM_SET_LOGGING
gclog_or_tty->print_cr("Adding " PTR_FORMAT " (" PTR_FORMAT ") to RS"
" for region [" PTR_FORMAT ", " PTR_FORMAT ")",
p, obj,
to->bottom(), to->end());
gclog_or_tty->print_cr("Adding " PTR_FORMAT " (" PTR_FORMAT ") to RS"
" for region [" PTR_FORMAT ", " PTR_FORMAT ")",
p, obj,
to->bottom(), to->end());
#endif
assert(to->rem_set() != NULL, "Need per-region 'into' remsets.");
to->rem_set()->add_reference(p, tid);
}
assert(to->rem_set() != NULL, "Need per-region 'into' remsets.");
to->rem_set()->add_reference(p, tid);
}
}
@ -108,3 +84,64 @@ inline void UpdateRSetImmediate::do_oop_work(T* p) {
}
}
template <class T>
inline void UpdateRSOrPushRefOopClosure::do_oop_work(T* p) {
oop obj = oopDesc::load_decode_heap_oop(p);
#ifdef ASSERT
// can't do because of races
// assert(obj == NULL || obj->is_oop(), "expected an oop");
// Do the safe subset of is_oop
if (obj != NULL) {
#ifdef CHECK_UNHANDLED_OOPS
oopDesc* o = obj.obj();
#else
oopDesc* o = obj;
#endif // CHECK_UNHANDLED_OOPS
assert((intptr_t)o % MinObjAlignmentInBytes == 0, "not oop aligned");
assert(Universe::heap()->is_in_reserved(obj), "must be in heap");
}
#endif // ASSERT
assert(_from != NULL, "from region must be non-NULL");
HeapRegion* to = _g1->heap_region_containing(obj);
if (to != NULL && _from != to) {
// The _record_refs_into_cset flag is true during the RSet
// updating part of an evacuation pause. It is false at all
// other times:
// * rebuilding the rembered sets after a full GC
// * during concurrent refinement.
// * updating the remembered sets of regions in the collection
// set in the event of an evacuation failure (when deferred
// updates are enabled).
if (_record_refs_into_cset && to->in_collection_set()) {
// We are recording references that point into the collection
// set and this particular reference does exactly that...
// If the referenced object has already been forwarded
// to itself, we are handling an evacuation failure and
// we have already visited/tried to copy this object
// there is no need to retry.
if (!self_forwarded(obj)) {
assert(_push_ref_cl != NULL, "should not be null");
// Push the reference in the refs queue of the G1ParScanThreadState
// instance for this worker thread.
_push_ref_cl->do_oop(p);
}
// Deferred updates to the CSet are either discarded (in the normal case),
// or processed (if an evacuation failure occurs) at the end
// of the collection.
// See G1RemSet::cleanup_after_oops_into_collection_set_do().
} else {
// We either don't care about pushing references that point into the
// collection set (i.e. we're not during an evacuation pause) _or_
// the reference doesn't point into the collection set. Either way
// we add the reference directly to the RSet of the region containing
// the referenced object.
_g1_rem_set->par_write_ref(_from, p, _worker_i);
}
}
}