# HG changeset patch # User johnc # Date 1335374592 25200 # Node ID d76458352d62d0593522c40aa3a0831a86b67c48 # Parent 6182ca66bc7b529caf1fe123845a059f4b509c25 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 diff -r 6182ca66bc7b -r d76458352d62 hotspot/src/share/vm/gc_implementation/g1/concurrentMark.cpp --- 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; } diff -r 6182ca66bc7b -r d76458352d62 hotspot/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp --- 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(); } } diff -r 6182ca66bc7b -r d76458352d62 hotspot/src/share/vm/gc_implementation/g1/g1CollectorPolicy.cpp --- 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; } } diff -r 6182ca66bc7b -r d76458352d62 hotspot/src/share/vm/gc_implementation/g1/heapRegion.cpp --- 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; } } diff -r 6182ca66bc7b -r d76458352d62 hotspot/src/share/vm/gc_implementation/g1/heapRegion.hpp --- 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; } } diff -r 6182ca66bc7b -r d76458352d62 hotspot/src/share/vm/gc_implementation/g1/heapRegion.inline.hpp --- 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(); }