7143490: G1: Remove HeapRegion::_top_at_conc_mark_count
Summary: Removed the HeapRegion::_top_at_conc_mark_count field. It is no longer needed as a result of the changes for 6888336 and 7127706. Refactored the closures that finalize and verify the liveness counting data so that common functionality was placed into a base class.
Reviewed-by: brutisso, tonyp
--- a/hotspot/src/share/vm/gc_implementation/g1/concurrentMark.cpp Thu Mar 29 19:46:24 2012 -0700
+++ b/hotspot/src/share/vm/gc_implementation/g1/concurrentMark.cpp Wed Apr 25 10:23:12 2012 -0700
@@ -1183,35 +1183,31 @@
g1p->record_concurrent_mark_remark_end();
}
-// Used to calculate the # live objects per region
-// for verification purposes
-class CalcLiveObjectsClosure: public HeapRegionClosure {
-
- CMBitMapRO* _bm;
+// Base class of the closures that finalize and verify the
+// liveness counting data.
+class CMCountDataClosureBase: public HeapRegionClosure {
+protected:
ConcurrentMark* _cm;
BitMap* _region_bm;
BitMap* _card_bm;
- size_t _region_marked_bytes;
-
- intptr_t _bottom_card_num;
-
- void mark_card_num_range(intptr_t start_card_num, intptr_t last_card_num) {
- assert(start_card_num <= last_card_num, "sanity");
- BitMap::idx_t start_idx = start_card_num - _bottom_card_num;
- BitMap::idx_t last_idx = last_card_num - _bottom_card_num;
-
- for (BitMap::idx_t i = start_idx; i <= last_idx; i += 1) {
- _card_bm->par_at_put(i, 1);
+ void set_card_bitmap_range(BitMap::idx_t start_idx, BitMap::idx_t last_idx) {
+ 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.
+ _card_bm->par_at_put_range(start_idx, last_idx+1, true);
}
}
-public:
- CalcLiveObjectsClosure(CMBitMapRO *bm, ConcurrentMark *cm,
- BitMap* region_bm, BitMap* card_bm) :
- _bm(bm), _cm(cm), _region_bm(region_bm), _card_bm(card_bm),
- _region_marked_bytes(0), _bottom_card_num(cm->heap_bottom_card_num()) { }
-
// 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
@@ -1234,6 +1230,24 @@
}
}
+public:
+ CMCountDataClosureBase(ConcurrentMark *cm,
+ BitMap* region_bm, BitMap* card_bm):
+ _cm(cm), _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,
+ BitMap* region_bm, BitMap* card_bm) :
+ CMCountDataClosureBase(cm, region_bm, card_bm),
+ _bm(bm), _region_marked_bytes(0) { }
+
bool doHeapRegion(HeapRegion* hr) {
if (hr->continuesHumongous()) {
@@ -1260,65 +1274,31 @@
size_t marked_bytes = 0;
- // Below, the term "card num" means the result of shifting an address
- // by the card shift -- address 0 corresponds to card number 0. One
- // must subtract the card num of the bottom of the heap to obtain a
- // card table index.
-
- // The first card num of the sequence of live cards currently being
- // constructed. -1 ==> no sequence.
- intptr_t start_card_num = -1;
-
- // The last card num of the sequence of live cards currently being
- // constructed. -1 ==> no sequence.
- intptr_t last_card_num = -1;
-
while (start < nextTop) {
oop obj = oop(start);
int obj_sz = obj->size();
-
- // The card num of the start of the current object.
- intptr_t obj_card_num =
- intptr_t(uintptr_t(start) >> CardTableModRefBS::card_shift);
HeapWord* obj_last = start + obj_sz - 1;
- intptr_t obj_last_card_num =
- intptr_t(uintptr_t(obj_last) >> CardTableModRefBS::card_shift);
-
- if (obj_card_num != last_card_num) {
- if (start_card_num == -1) {
- assert(last_card_num == -1, "Both or neither.");
- start_card_num = obj_card_num;
- } else {
- assert(last_card_num != -1, "Both or neither.");
- assert(obj_card_num >= last_card_num, "Inv");
- if ((obj_card_num - last_card_num) > 1) {
- // Mark the last run, and start a new one.
- mark_card_num_range(start_card_num, last_card_num);
- start_card_num = obj_card_num;
- }
- }
- }
- // In any case, we set the last card num.
- last_card_num = obj_last_card_num;
-
+
+ BitMap::idx_t start_idx = _cm->card_bitmap_index_for(start);
+ BitMap::idx_t last_idx = _cm->card_bitmap_index_for(obj_last);
+
+ // Set the bits in the card BM for this object (inclusive).
+ set_card_bitmap_range(start_idx, last_idx);
+
+ // 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(start + 1, nextTop);
- }
-
- // Handle the last range, if any.
- if (start_card_num != -1) {
- mark_card_num_range(start_card_num, last_card_num);
+ start = _bm->getNextMarkedWordAddress(obj_last + 1, nextTop);
}
// Mark the allocated-since-marking portion...
HeapWord* top = hr->top();
if (nextTop < top) {
- start_card_num = intptr_t(uintptr_t(nextTop) >> CardTableModRefBS::card_shift);
- last_card_num = intptr_t(uintptr_t(top) >> CardTableModRefBS::card_shift);
-
- mark_card_num_range(start_card_num, last_card_num);
+ BitMap::idx_t start_idx = _cm->card_bitmap_index_for(nextTop);
+ BitMap::idx_t last_idx = _cm->card_bitmap_index_for(top - 1);
+
+ set_card_bitmap_range(start_idx, last_idx);
// This definitely means the region has live objects.
set_bit_for_region(hr);
@@ -1394,17 +1374,6 @@
MutexLockerEx x((_verbose ? ParGCRareEvent_lock : NULL),
Mutex::_no_safepoint_check_flag);
- // Verify that _top_at_conc_count == ntams
- if (hr->top_at_conc_mark_count() != hr->next_top_at_mark_start()) {
- if (_verbose) {
- gclog_or_tty->print_cr("Region %u: top at conc count incorrect: "
- "expected " PTR_FORMAT ", actual: " PTR_FORMAT,
- hr->hrs_index(), hr->next_top_at_mark_start(),
- hr->top_at_conc_mark_count());
- }
- failures += 1;
- }
-
// Verify the marked bytes for this region.
size_t exp_marked_bytes = _calc_cl.region_marked_bytes();
size_t act_marked_bytes = hr->next_marked_bytes();
@@ -1470,7 +1439,7 @@
_failures += failures;
// We could stop iteration over the heap when we
- // find the first voilating region by returning true.
+ // find the first violating region by returning true.
return false;
}
};
@@ -1543,62 +1512,19 @@
int failures() const { return _failures; }
};
-// Final update of count data (during cleanup).
-// Adds [top_at_count, NTAMS) to the marked bytes for each
-// region. Sets the bits in the card bitmap corresponding
-// to the interval [top_at_count, top], and sets the
-// liveness bit for each region containing live data
-// in the region bitmap.
-
-class FinalCountDataUpdateClosure: public HeapRegionClosure {
- ConcurrentMark* _cm;
- BitMap* _region_bm;
- BitMap* _card_bm;
-
- void set_card_bitmap_range(BitMap::idx_t start_idx, BitMap::idx_t last_idx) {
- 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.
- _card_bm->par_at_put_range(start_idx, last_idx+1, 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) {
- assert(!hr->continuesHumongous(), "should have filtered those out");
-
- BitMap::idx_t index = (BitMap::idx_t) hr->hrs_index();
- if (!hr->startsHumongous()) {
- // Normal (non-humongous) case: just set the bit.
- _region_bm->par_set_bit(index);
- } else {
- // Starts humongous case: calculate how many regions are part of
- // this humongous region and then set the bit range.
- G1CollectedHeap* g1h = G1CollectedHeap::heap();
- HeapRegion *last_hr = g1h->heap_region_containing_raw(hr->end() - 1);
- BitMap::idx_t end_index = (BitMap::idx_t) last_hr->hrs_index() + 1;
- _region_bm->par_at_put_range(index, end_index, true);
- }
- }
-
+// Closure that finalizes the liveness counting data.
+// Used during the cleanup pause.
+// Sets the bits corresponding to the interval [NTAMS, top]
+// (which contains the implicitly live objects) in the
+// 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,
BitMap* region_bm,
BitMap* card_bm) :
- _cm(cm), _region_bm(region_bm), _card_bm(card_bm) { }
+ CMCountDataClosureBase(cm, region_bm, card_bm) { }
bool doHeapRegion(HeapRegion* hr) {
@@ -1613,26 +1539,10 @@
return false;
}
- HeapWord* start = hr->top_at_conc_mark_count();
HeapWord* ntams = hr->next_top_at_mark_start();
HeapWord* top = hr->top();
- assert(hr->bottom() <= start && start <= hr->end() &&
- hr->bottom() <= ntams && ntams <= hr->end(), "Preconditions.");
-
- if (start < ntams) {
- // Region was changed between remark and cleanup pauses
- // We need to add (ntams - start) to the marked bytes
- // for this region, and set bits for the range
- // [ card_idx(start), card_idx(ntams) ) in the card bitmap.
- size_t live_bytes = (ntams - start) * HeapWordSize;
- hr->add_to_marked_bytes(live_bytes);
-
- // Record the new top at conc count
- hr->set_top_at_conc_mark_count(ntams);
-
- // The setting of the bits in the card bitmap takes place below
- }
+ assert(hr->bottom() <= ntams && ntams <= hr->end(), "Preconditions.");
// Mark the allocated-since-marking portion...
if (ntams < top) {
@@ -1640,8 +1550,8 @@
set_bit_for_region(hr);
}
- // Now set the bits for [start, top]
- BitMap::idx_t start_idx = _cm->card_bitmap_index_for(start);
+ // Now set the bits for [ntams, top]
+ BitMap::idx_t start_idx = _cm->card_bitmap_index_for(ntams);
BitMap::idx_t last_idx = _cm->card_bitmap_index_for(top);
set_card_bitmap_range(start_idx, last_idx);
@@ -3072,9 +2982,6 @@
// Update the marked bytes for this region.
hr->add_to_marked_bytes(marked_bytes);
- // Now set the top at count to NTAMS.
- hr->set_top_at_conc_mark_count(limit);
-
// Next heap region
return false;
}
--- a/hotspot/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp Thu Mar 29 19:46:24 2012 -0700
+++ b/hotspot/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp Wed Apr 25 10:23:12 2012 -0700
@@ -368,16 +368,11 @@
if (curr == NULL)
gclog_or_tty->print_cr(" empty");
while (curr != NULL) {
- gclog_or_tty->print_cr(" [%08x-%08x], t: %08x, P: %08x, N: %08x, C: %08x, "
- "age: %4d, y: %d, surv: %d",
- curr->bottom(), curr->end(),
- curr->top(),
+ gclog_or_tty->print_cr(" "HR_FORMAT", P: "PTR_FORMAT "N: "PTR_FORMAT", age: %4d",
+ HR_FORMAT_PARAMS(curr),
curr->prev_top_at_mark_start(),
curr->next_top_at_mark_start(),
- curr->top_at_conc_mark_count(),
- curr->age_in_surv_rate_group_cond(),
- curr->is_young(),
- curr->is_survivor());
+ curr->age_in_surv_rate_group_cond());
curr = curr->get_next_young_region();
}
}
--- a/hotspot/src/share/vm/gc_implementation/g1/g1CollectorPolicy.cpp Thu Mar 29 19:46:24 2012 -0700
+++ b/hotspot/src/share/vm/gc_implementation/g1/g1CollectorPolicy.cpp Wed Apr 25 10:23:12 2012 -0700
@@ -2459,16 +2459,10 @@
while (csr != NULL) {
HeapRegion* next = csr->next_in_collection_set();
assert(csr->in_collection_set(), "bad CS");
- st->print_cr(" [%08x-%08x], t: %08x, P: %08x, N: %08x, C: %08x, "
- "age: %4d, y: %d, surv: %d",
- csr->bottom(), csr->end(),
- csr->top(),
- csr->prev_top_at_mark_start(),
- csr->next_top_at_mark_start(),
- csr->top_at_conc_mark_count(),
- csr->age_in_surv_rate_group_cond(),
- csr->is_young(),
- csr->is_survivor());
+ st->print_cr(" "HR_FORMAT", P: "PTR_FORMAT "N: "PTR_FORMAT", age: %4d",
+ HR_FORMAT_PARAMS(csr),
+ csr->prev_top_at_mark_start(), csr->next_top_at_mark_start(),
+ csr->age_in_surv_rate_group_cond());
csr = next;
}
}
--- a/hotspot/src/share/vm/gc_implementation/g1/heapRegion.cpp Thu Mar 29 19:46:24 2012 -0700
+++ b/hotspot/src/share/vm/gc_implementation/g1/heapRegion.cpp Wed Apr 25 10:23:12 2012 -0700
@@ -510,9 +510,6 @@
_rem_set = new HeapRegionRemSet(sharedOffsetArray, this);
assert(HeapRegionRemSet::num_par_rem_sets() > 0, "Invariant.");
- // In case the region is allocated during a pause, note the top.
- // We haven't done any counting on a brand new region.
- _top_at_conc_mark_count = bottom();
}
class NextCompactionHeapRegionClosure: public HeapRegionClosure {
@@ -585,14 +582,12 @@
// we find to be self-forwarded on the next bitmap. So all
// objects need to be below NTAMS.
_next_top_at_mark_start = top();
- set_top_at_conc_mark_count(bottom());
_next_marked_bytes = 0;
} else if (during_conc_mark) {
// During concurrent mark, all objects in the CSet (including
// the ones we find to be self-forwarded) are implicitly live.
// So all objects need to be above NTAMS.
_next_top_at_mark_start = bottom();
- set_top_at_conc_mark_count(bottom());
_next_marked_bytes = 0;
}
}
--- a/hotspot/src/share/vm/gc_implementation/g1/heapRegion.hpp Thu Mar 29 19:46:24 2012 -0700
+++ b/hotspot/src/share/vm/gc_implementation/g1/heapRegion.hpp Wed Apr 25 10:23:12 2012 -0700
@@ -306,9 +306,6 @@
// If a collection pause is in progress, this is the top at the start
// of that pause.
- // We've counted the marked bytes of objects below here.
- HeapWord* _top_at_conc_mark_count;
-
void init_top_at_mark_start() {
assert(_prev_marked_bytes == 0 &&
_next_marked_bytes == 0,
@@ -316,7 +313,6 @@
HeapWord* bot = bottom();
_prev_top_at_mark_start = bot;
_next_top_at_mark_start = bot;
- _top_at_conc_mark_count = bot;
}
void set_young_type(YoungType new_type) {
@@ -625,19 +621,6 @@
// last mark phase ended.
bool is_marked() { return _prev_top_at_mark_start != bottom(); }
- void init_top_at_conc_mark_count() {
- _top_at_conc_mark_count = bottom();
- }
-
- void set_top_at_conc_mark_count(HeapWord *cur) {
- assert(bottom() <= cur && cur <= end(), "Sanity.");
- _top_at_conc_mark_count = cur;
- }
-
- HeapWord* top_at_conc_mark_count() {
- return _top_at_conc_mark_count;
- }
-
void reset_during_compaction() {
guarantee( isHumongous() && startsHumongous(),
"should only be called for humongous regions");
@@ -733,7 +716,6 @@
_evacuation_failed = b;
if (b) {
- init_top_at_conc_mark_count();
_next_marked_bytes = 0;
}
}
--- a/hotspot/src/share/vm/gc_implementation/g1/heapRegion.inline.hpp Thu Mar 29 19:46:24 2012 -0700
+++ b/hotspot/src/share/vm/gc_implementation/g1/heapRegion.inline.hpp Wed Apr 25 10:23:12 2012 -0700
@@ -56,7 +56,6 @@
}
inline void HeapRegion::note_start_of_marking() {
- init_top_at_conc_mark_count();
_next_marked_bytes = 0;
_next_top_at_mark_start = top();
}