truffle: src/share/vm/gc_implementation/g1/concurrentMark.cpp comparison

comparison src/share/vm/gc_implementation/g1/concurrentMark.cpp @ 6812:988bf00cc564

7200261: G1: Liveness counting inconsistencies during marking verification Summary: The clipping code in the routine that sets the bits for a range of cards, in the liveness accounting verification code was incorrect. It set all the bits in the card bitmap from the given starting index which would lead to spurious marking verification failures. Reviewed-by: brutisso, jwilhelm, jmasa

author	johnc
date	Thu, 27 Sep 2012 15:44:01 -0700
parents	859cd1a76f8a
children	8a5ea0a9ccc4

comparison

equal deleted inserted replaced

-:7c2fd5948145
+:988bf00cc564
 // Base class of the closures that finalize and verify the
 // liveness counting data.
 class CMCountDataClosureBase: public HeapRegionClosure {
 protected:
+G1CollectedHeap* _g1h;
 ConcurrentMark* _cm;
+CardTableModRefBS* _ct_bs;
 BitMap* _region_bm;
 BitMap* _card_bm;
-void set_card_bitmap_range(BitMap::idx_t start_idx, BitMap::idx_t last_idx) {
+// Takes a region that's not empty (i.e., it has at least one
-assert(start_idx <= last_idx, "sanity");
-// Set the inclusive bit range [start_idx, last_idx].
-// For small ranges (up to 8 cards) use a simple loop; otherwise
-// use par_at_put_range.
-if ((last_idx - start_idx) < 8) {
-for (BitMap::idx_t i = start_idx; i <= last_idx; i += 1) {
-_card_bm->par_set_bit(i);
-}
-} else {
-assert(last_idx < _card_bm->size(), "sanity");
-// Note BitMap::par_at_put_range() is exclusive.
-BitMap::idx_t max_idx = MAX2(last_idx+1, _card_bm->size());
-_card_bm->par_at_put_range(start_idx, max_idx, true);
-}
-}
-// It takes a region that's not empty (i.e., it has at least one
 // live object in it and sets its corresponding bit on the region
 // bitmap to 1. If the region is "starts humongous" it will also set
 // to 1 the bits on the region bitmap that correspond to its
 // associated "continues humongous" regions.
 void set_bit_for_region(HeapRegion* hr) {
 _region_bm->par_at_put_range(index, end_index, true);
 }
 }
 public:
-CMCountDataClosureBase(ConcurrentMark *cm,
+CMCountDataClosureBase(G1CollectedHeap* g1h,
 BitMap* region_bm, BitMap* card_bm):
-_cm(cm), _region_bm(region_bm), _card_bm(card_bm) { }
+_g1h(g1h), _cm(g1h->concurrent_mark()),
+_ct_bs((CardTableModRefBS*) (g1h->barrier_set())),
+_region_bm(region_bm), _card_bm(card_bm) { }
 };
 // Closure that calculates the # live objects per region. Used
 // for verification purposes during the cleanup pause.
 class CalcLiveObjectsClosure: public CMCountDataClosureBase {
 CMBitMapRO* _bm;
 size_t _region_marked_bytes;
 public:
-CalcLiveObjectsClosure(CMBitMapRO *bm, ConcurrentMark *cm,
+CalcLiveObjectsClosure(CMBitMapRO *bm, G1CollectedHeap* g1h,
 BitMap* region_bm, BitMap* card_bm) :
-CMCountDataClosureBase(cm, region_bm, card_bm),
+CMCountDataClosureBase(g1h, region_bm, card_bm),
 _bm(bm), _region_marked_bytes(0) { }
 bool doHeapRegion(HeapRegion* hr) {
 if (hr->continuesHumongous()) {
 // iteration, a "continues humongous" region might be visited
 // before its associated "starts humongous".
 return false;
 }
-HeapWord* nextTop = hr->next_top_at_mark_start();
+HeapWord* ntams = hr->next_top_at_mark_start();
-HeapWord* start   = hr->bottom();
+HeapWord* start = hr->bottom();
-assert(start <= hr->end() && start <= nextTop && nextTop <= hr->end(),
+assert(start <= hr->end() && start <= ntams && ntams <= hr->end(),
 err_msg("Preconditions not met - "
-"start: "PTR_FORMAT", nextTop: "PTR_FORMAT", end: "PTR_FORMAT,
+"start: "PTR_FORMAT", ntams: "PTR_FORMAT", end: "PTR_FORMAT,
-start, nextTop, hr->end()));
+start, ntams, hr->end()));
 // Find the first marked object at or after "start".
-start = _bm->getNextMarkedWordAddress(start, nextTop);
+start = _bm->getNextMarkedWordAddress(start, ntams);
 size_t marked_bytes = 0;
-while (start < nextTop) {
+while (start < ntams) {
 oop obj = oop(start);
 int obj_sz = obj->size();
-HeapWord* obj_last = start + obj_sz - 1;
+HeapWord* obj_end = start + obj_sz;
 BitMap::idx_t start_idx = _cm->card_bitmap_index_for(start);
-BitMap::idx_t last_idx = _cm->card_bitmap_index_for(obj_last);
+BitMap::idx_t end_idx = _cm->card_bitmap_index_for(obj_end);
-// Set the bits in the card BM for this object (inclusive).
+// Note: if we're looking at the last region in heap - obj_end
-set_card_bitmap_range(start_idx, last_idx);
+// could be actually just beyond the end of the heap; end_idx
+// will then correspond to a (non-existent) card that is also
+// just beyond the heap.
+if (_g1h->is_in_g1_reserved(obj_end) && !_ct_bs->is_card_aligned(obj_end)) {
+// end of object is not card aligned - increment to cover
+// all the cards spanned by the object
+end_idx += 1;
+}
+// Set the bits in the card BM for the cards spanned by this object.
+_cm->set_card_bitmap_range(_card_bm, start_idx, end_idx, true /* is_par */);
 // Add the size of this object to the number of marked bytes.
 marked_bytes += (size_t)obj_sz * HeapWordSize;
 // Find the next marked object after this one.
-start = _bm->getNextMarkedWordAddress(obj_last + 1, nextTop);
+start = _bm->getNextMarkedWordAddress(obj_end, ntams);
 }
 // Mark the allocated-since-marking portion...
 HeapWord* top = hr->top();
-if (nextTop < top) {
+if (ntams < top) {
-BitMap::idx_t start_idx = _cm->card_bitmap_index_for(nextTop);
+BitMap::idx_t start_idx = _cm->card_bitmap_index_for(ntams);
-BitMap::idx_t last_idx = _cm->card_bitmap_index_for(top - 1);
+BitMap::idx_t end_idx = _cm->card_bitmap_index_for(top);
-set_card_bitmap_range(start_idx, last_idx);
+// Note: if we're looking at the last region in heap - top
+// could be actually just beyond the end of the heap; end_idx
+// will then correspond to a (non-existent) card that is also
+// just beyond the heap.
+if (_g1h->is_in_g1_reserved(top) && !_ct_bs->is_card_aligned(top)) {
+// end of object is not card aligned - increment to cover
+// all the cards spanned by the object
+end_idx += 1;
+}
+_cm->set_card_bitmap_range(_card_bm, start_idx, end_idx, true /* is_par */);
 // This definitely means the region has live objects.
 set_bit_for_region(hr);
 }
 // that was accumulated concurrently and aggregated during
 // the remark pause. This closure is applied to the heap
 // regions during the STW cleanup pause.
 class VerifyLiveObjectDataHRClosure: public HeapRegionClosure {
+G1CollectedHeap* _g1h;
 ConcurrentMark* _cm;
 CalcLiveObjectsClosure _calc_cl;
 BitMap* _region_bm;   // Region BM to be verified
 BitMap* _card_bm;     // Card BM to be verified
 bool _verbose;        // verbose output?
 BitMap* _exp_card_bm;   // Expected card BM values
 int _failures;
 public:
-VerifyLiveObjectDataHRClosure(ConcurrentMark* cm,
+VerifyLiveObjectDataHRClosure(G1CollectedHeap* g1h,
 BitMap* region_bm,
 BitMap* card_bm,
 BitMap* exp_region_bm,
 BitMap* exp_card_bm,
 bool verbose) :
-_cm(cm),
+_g1h(g1h), _cm(g1h->concurrent_mark()),
-_calc_cl(_cm->nextMarkBitMap(), _cm, exp_region_bm, exp_card_bm),
+_calc_cl(_cm->nextMarkBitMap(), g1h, exp_region_bm, exp_card_bm),
 _region_bm(region_bm), _card_bm(card_bm), _verbose(verbose),
 _exp_region_bm(exp_region_bm), _exp_card_bm(exp_card_bm),
 _failures(0) { }
 int failures() const { return _failures; }
 }
 void work(uint worker_id) {
 assert(worker_id < _n_workers, "invariant");
-VerifyLiveObjectDataHRClosure verify_cl(_cm,
+VerifyLiveObjectDataHRClosure verify_cl(_g1h,
 _actual_region_bm, _actual_card_bm,
 _expected_region_bm,
 _expected_card_bm,
 _verbose);
 // card liveness bitmap. Also sets the bit for each region,
 // containing live data, in the region liveness bitmap.
 class FinalCountDataUpdateClosure: public CMCountDataClosureBase {
 public:
-FinalCountDataUpdateClosure(ConcurrentMark* cm,
+FinalCountDataUpdateClosure(G1CollectedHeap* g1h,
 BitMap* region_bm,
 BitMap* card_bm) :
-CMCountDataClosureBase(cm, region_bm, card_bm) { }
+CMCountDataClosureBase(g1h, region_bm, card_bm) { }
 bool doHeapRegion(HeapRegion* hr) {
 if (hr->continuesHumongous()) {
 // We will ignore these here and process them when their
 // Mark the allocated-since-marking portion...
 if (ntams < top) {
 // This definitely means the region has live objects.
 set_bit_for_region(hr);
-}
+// Now set the bits in the card bitmap for [ntams, top)
-// Now set the bits for [ntams, top]
+BitMap::idx_t start_idx = _cm->card_bitmap_index_for(ntams);
-BitMap::idx_t start_idx = _cm->card_bitmap_index_for(ntams);
+BitMap::idx_t end_idx = _cm->card_bitmap_index_for(top);
-// set_card_bitmap_range() expects the last_idx to be with
-// the range of the bit map (see assertion in set_card_bitmap_range()),
+// Note: if we're looking at the last region in heap - top
-// so limit it to that range with this application of MIN2.
+// could be actually just beyond the end of the heap; end_idx
-BitMap::idx_t last_idx = MIN2(_cm->card_bitmap_index_for(top),
+// will then correspond to a (non-existent) card that is also
-_card_bm->size()-1);
+// just beyond the heap.
-if (start_idx < _card_bm->size()) {
+if (_g1h->is_in_g1_reserved(top) && !_ct_bs->is_card_aligned(top)) {
-set_card_bitmap_range(start_idx, last_idx);
+// end of object is not card aligned - increment to cover
-} else {
+// all the cards spanned by the object
-// To reach here start_idx must be beyond the end of
+end_idx += 1;
-// the bit map and last_idx must have been limited by
+}
-// the MIN2().
-assert(start_idx == last_idx + 1,
+assert(end_idx <= _card_bm->size(),
-err_msg("Not beyond end start_idx " SIZE_FORMAT " last_idx "
+err_msg("oob: end_idx=  "SIZE_FORMAT", bitmap size= "SIZE_FORMAT,
-SIZE_FORMAT, start_idx, last_idx));
+end_idx, _card_bm->size()));
+assert(start_idx < _card_bm->size(),
+err_msg("oob: start_idx=  "SIZE_FORMAT", bitmap size= "SIZE_FORMAT,
+start_idx, _card_bm->size()));
+_cm->set_card_bitmap_range(_card_bm, start_idx, end_idx, true /* is_par */);
 }
 // Set the bit for the region if it contains live data
 if (hr->next_marked_bytes() > 0) {
 set_bit_for_region(hr);
 }
 void work(uint worker_id) {
 assert(worker_id < _n_workers, "invariant");
-FinalCountDataUpdateClosure final_update_cl(_cm,
+FinalCountDataUpdateClosure final_update_cl(_g1h,
 _actual_region_bm,
 _actual_card_bm);
 if (G1CollectedHeap::use_parallel_gc_threads()) {
 _g1h->heap_region_par_iterate_chunked(&final_update_cl,
 }
 // Aggregate the counting data that was constructed concurrently
 // with marking.
 class AggregateCountDataHRClosure: public HeapRegionClosure {
+G1CollectedHeap* _g1h;
 ConcurrentMark* _cm;
+CardTableModRefBS* _ct_bs;
 BitMap* _cm_card_bm;
 size_t _max_task_num;
 public:
-AggregateCountDataHRClosure(ConcurrentMark *cm,
+AggregateCountDataHRClosure(G1CollectedHeap* g1h,
 BitMap* cm_card_bm,
 size_t max_task_num) :
-_cm(cm), _cm_card_bm(cm_card_bm),
+_g1h(g1h), _cm(g1h->concurrent_mark()),
-_max_task_num(max_task_num) { }
+_ct_bs((CardTableModRefBS*) (g1h->barrier_set())),
+_cm_card_bm(cm_card_bm), _max_task_num(max_task_num) { }
-bool is_card_aligned(HeapWord* p) {
-return ((uintptr_t(p) & (CardTableModRefBS::card_size - 1)) == 0);
-}
 bool doHeapRegion(HeapRegion* hr) {
 if (hr->continuesHumongous()) {
 // We will ignore these here and process them when their
 // associated "starts humongous" region is processed.
 if (start == limit) {
 // NTAMS of this region has not been set so nothing to do.
 return false;
 }
-assert(is_card_aligned(start), "sanity");
+// 'start' should be in the heap.
-assert(is_card_aligned(end), "sanity");
+assert(_g1h->is_in_g1_reserved(start) && _ct_bs->is_card_aligned(start), "sanity");
+// 'end' *may* be just beyone the end of the heap (if hr is the last region)
+assert(!_g1h->is_in_g1_reserved(end) || _ct_bs->is_card_aligned(end), "sanity");
 BitMap::idx_t start_idx = _cm->card_bitmap_index_for(start);
 BitMap::idx_t limit_idx = _cm->card_bitmap_index_for(limit);
 BitMap::idx_t end_idx = _cm->card_bitmap_index_for(end);
-// If ntams is not card aligned then we bump the index for
+// If ntams is not card aligned then we bump card bitmap index
-// limit so that we get the card spanning ntams.
+// for limit so that we get the all the cards spanned by
-if (!is_card_aligned(limit)) {
+// the object ending at ntams.
+// Note: if this is the last region in the heap then ntams
+// could be actually just beyond the end of the the heap;
+// limit_idx will then  correspond to a (non-existent) card
+// that is also outside the heap.
+if (_g1h->is_in_g1_reserved(limit) && !_ct_bs->is_card_aligned(limit)) {
 limit_idx += 1;
 }
 assert(limit_idx <= end_idx, "or else use atomics");
 _cm_card_bm->set_bit(scan_idx);
 assert(_cm_card_bm->at(scan_idx) == true, "should be");
 // BitMap::get_next_one_offset() can handle the case when
 // its left_offset parameter is greater than its right_offset
-// parameter. If does, however, have an early exit if
+// parameter. It does, however, have an early exit if
 // left_offset == right_offset. So let's limit the value
 // passed in for left offset here.
 BitMap::idx_t next_idx = MIN2(scan_idx + 1, limit_idx);
 scan_idx = task_card_bm->get_next_one_offset(next_idx, limit_idx);
 }
 _g1h(g1h), _cm(cm), _cm_card_bm(cm_card_bm),
 _max_task_num(max_task_num),
 _active_workers(n_workers) { }
 void work(uint worker_id) {
-AggregateCountDataHRClosure cl(_cm, _cm_card_bm, _max_task_num);
+AggregateCountDataHRClosure cl(_g1h, _cm_card_bm, _max_task_num);
 if (G1CollectedHeap::use_parallel_gc_threads()) {
 _g1h->heap_region_par_iterate_chunked(&cl, worker_id,
 _active_workers,
 HeapRegion::AggregateCountClaimValue);

Mercurial > hg > truffle

comparison src/share/vm/gc_implementation/g1/concurrentMark.cpp @ 6812:988bf00cc564