8011724: G1: Stack allocate instances of HeapRegionRemSetIterator

Summary: Stack allocate instances of HeapRegionRemSetIterator during RSet scanning.
Reviewed-by: brutisso, jwilhelm

src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp

@@ -1955,13 +1955,6 @@ G1CollectedHeap::G1CollectedHeap(G1CollectorPolicy* policy_) :
   int n_rem_sets = HeapRegionRemSet::num_par_rem_sets();
   assert(n_rem_sets > 0, "Invariant.");
 
-  HeapRegionRemSetIterator** iter_arr =
-    NEW_C_HEAP_ARRAY(HeapRegionRemSetIterator*, n_queues, mtGC);
-  for (int i = 0; i < n_queues; i++) {
-    iter_arr[i] = new HeapRegionRemSetIterator();
-  }
-  _rem_set_iterator = iter_arr;
-
   _worker_cset_start_region = NEW_C_HEAP_ARRAY(HeapRegion*, n_queues, mtGC);
   _worker_cset_start_region_time_stamp = NEW_C_HEAP_ARRAY(unsigned int, n_queues, mtGC);
 

src/share/vm/gc_implementation/g1/g1CollectedHeap.hpp

@@ -786,9 +786,6 @@ protected:
   // concurrently after the collection.
   DirtyCardQueueSet _dirty_card_queue_set;
 
-  // The Heap Region Rem Set Iterator.
-  HeapRegionRemSetIterator** _rem_set_iterator;
-
   // The closure used to refine a single card.
   RefineCardTableEntryClosure* _refine_cte_cl;
 
@@ -1113,15 +1110,6 @@ public:
   G1RemSet* g1_rem_set() const { return _g1_rem_set; }
   ModRefBarrierSet* mr_bs() const { return _mr_bs; }
 
-  // The rem set iterator.
-  HeapRegionRemSetIterator* rem_set_iterator(int i) {
-    return _rem_set_iterator[i];
-  }
-
-  HeapRegionRemSetIterator* rem_set_iterator() {
-    return _rem_set_iterator[0];
-  }
-
   unsigned get_gc_time_stamp() {
     return _gc_time_stamp;
   }

src/share/vm/gc_implementation/g1/g1RemSet.cpp

@@ -169,14 +169,13 @@ public:
     //   _try_claimed || r->claim_iter()
     // is true: either we're supposed to work on claimed-but-not-complete
     // regions, or we successfully claimed the region.
-    HeapRegionRemSetIterator* iter = _g1h->rem_set_iterator(_worker_i);
-    hrrs->init_iterator(iter);
+    HeapRegionRemSetIterator iter(hrrs);
     size_t card_index;
 
     // We claim cards in block so as to recude the contention. The block size is determined by
     // the G1RSetScanBlockSize parameter.
     size_t jump_to_card = hrrs->iter_claimed_next(_block_size);
-    for (size_t current_card = 0; iter->has_next(card_index); current_card++) {
+    for (size_t current_card = 0; iter.has_next(card_index); current_card++) {
       if (current_card >= jump_to_card + _block_size) {
         jump_to_card = hrrs->iter_claimed_next(_block_size);
       }

src/share/vm/gc_implementation/g1/g1RemSet.hpp

@@ -53,14 +53,14 @@ protected:
     NumSeqTasks          = 1
   };
 
-  CardTableModRefBS*             _ct_bs;
-  SubTasksDone*                  _seq_task;
-  G1CollectorPolicy* _g1p;
+  CardTableModRefBS*     _ct_bs;
+  SubTasksDone*          _seq_task;
+  G1CollectorPolicy*     _g1p;
 
-  ConcurrentG1Refine* _cg1r;
+  ConcurrentG1Refine*    _cg1r;
 
-  size_t*             _cards_scanned;
-  size_t              _total_cards_scanned;
+  size_t*                _cards_scanned;
+  size_t                 _total_cards_scanned;
 
   // Used for caching the closure that is responsible for scanning
   // references into the collection set.

src/share/vm/gc_implementation/g1/heapRegionRemSet.cpp

@@ -877,14 +877,9 @@ bool HeapRegionRemSet::iter_is_complete() {
   return _iter_state == Complete;
 }
 
-void HeapRegionRemSet::init_iterator(HeapRegionRemSetIterator* iter) const {
-  iter->initialize(this);
-}
-
 #ifndef PRODUCT
 void HeapRegionRemSet::print() const {
-  HeapRegionRemSetIterator iter;
-  init_iterator(&iter);
+  HeapRegionRemSetIterator iter(this);
   size_t card_index;
   while (iter.has_next(card_index)) {
     HeapWord* card_start =
@@ -928,35 +923,23 @@ void HeapRegionRemSet::scrub(CardTableModRefBS* ctbs,
 
 //-------------------- Iteration --------------------
 
-HeapRegionRemSetIterator::
-HeapRegionRemSetIterator() :
-  _hrrs(NULL),
+HeapRegionRemSetIterator:: HeapRegionRemSetIterator(const HeapRegionRemSet* hrrs) :
+  _hrrs(hrrs),
   _g1h(G1CollectedHeap::heap()),
-  _bosa(NULL),
-  _sparse_iter() { }
-
-void HeapRegionRemSetIterator::initialize(const HeapRegionRemSet* hrrs) {
-  _hrrs = hrrs;
-  _coarse_map = &_hrrs->_other_regions._coarse_map;
-  _fine_grain_regions = _hrrs->_other_regions._fine_grain_regions;
-  _bosa = _hrrs->bosa();
-
-  _is = Sparse;
+  _coarse_map(&hrrs->_other_regions._coarse_map),
+  _fine_grain_regions(hrrs->_other_regions._fine_grain_regions),
+  _bosa(hrrs->bosa()),
+  _is(Sparse),
   // Set these values so that we increment to the first region.
-  _coarse_cur_region_index = -1;
-  _coarse_cur_region_cur_card = (HeapRegion::CardsPerRegion-1);
-
-  _cur_region_cur_card = 0;
-
-  _fine_array_index = -1;
-  _fine_cur_prt = NULL;
-
-  _n_yielded_coarse = 0;
-  _n_yielded_fine = 0;
-  _n_yielded_sparse = 0;
-
-  _sparse_iter.init(&hrrs->_other_regions._sparse_table);
-}
+  _coarse_cur_region_index(-1),
+  _coarse_cur_region_cur_card(HeapRegion::CardsPerRegion-1),
+  _cur_region_cur_card(0),
+  _fine_array_index(-1),
+  _fine_cur_prt(NULL),
+  _n_yielded_coarse(0),
+  _n_yielded_fine(0),
+  _n_yielded_sparse(0),
+  _sparse_iter(&hrrs->_other_regions._sparse_table) {}
 
 bool HeapRegionRemSetIterator::coarse_has_next(size_t& card_index) {
   if (_hrrs->_other_regions._n_coarse_entries == 0) return false;
@@ -1209,8 +1192,7 @@ void HeapRegionRemSet::test() {
   hrrs->add_reference((OopOrNarrowOopStar)hr5->bottom());
 
   // Now, does iteration yield these three?
-  HeapRegionRemSetIterator iter;
-  hrrs->init_iterator(&iter);
+  HeapRegionRemSetIterator iter(hrrs);
   size_t sum = 0;
   size_t card_index;
   while (iter.has_next(card_index)) {

src/share/vm/gc_implementation/g1/heapRegionRemSet.hpp

@@ -281,9 +281,6 @@ public:
     return (_iter_state == Unclaimed) && (_iter_claimed == 0);
   }
 
-  // Initialize the given iterator to iterate over this rem set.
-  void init_iterator(HeapRegionRemSetIterator* iter) const;
-
   // The actual # of bytes this hr_remset takes up.
   size_t mem_size() {
     return _other_regions.mem_size()
@@ -345,9 +342,9 @@ public:
 #endif
 };
 
-class HeapRegionRemSetIterator : public CHeapObj<mtGC> {
+class HeapRegionRemSetIterator : public StackObj {
 
-  // The region over which we're iterating.
+  // The region RSet over which we're iterating.
   const HeapRegionRemSet* _hrrs;
 
   // Local caching of HRRS fields.
@@ -362,8 +359,10 @@ class HeapRegionRemSetIterator : public CHeapObj<mtGC> {
   size_t _n_yielded_coarse;
   size_t _n_yielded_sparse;
 
-  // If true we're iterating over the coarse table; if false the fine
-  // table.
+  // Indicates what granularity of table that we're currently iterating over.
+  // We start iterating over the sparse table, progress to the fine grain
+  // table, and then finish with the coarse table.
+  // See HeapRegionRemSetIterator::has_next().
   enum IterState {
     Sparse,
     Fine,
@@ -403,9 +402,7 @@ class HeapRegionRemSetIterator : public CHeapObj<mtGC> {
 public:
   // We require an iterator to be initialized before use, so the
   // constructor does little.
-  HeapRegionRemSetIterator();
-
-  void initialize(const HeapRegionRemSet* hrrs);
+  HeapRegionRemSetIterator(const HeapRegionRemSet* hrrs);
 
   // If there remains one or more cards to be yielded, returns true and
   // sets "card_index" to one of those cards (which is then considered

src/share/vm/gc_implementation/g1/sparsePRT.cpp

@@ -35,10 +35,6 @@
 
 #define UNROLL_CARD_LOOPS  1
 
-void SparsePRT::init_iterator(SparsePRTIter* sprt_iter) {
-    sprt_iter->init(this);
-}
-
 void SparsePRTEntry::init(RegionIdx_t region_ind) {
   _region_ind = region_ind;
   _next_index = NullEntry;

src/share/vm/gc_implementation/g1/sparsePRT.hpp

@@ -192,18 +192,11 @@ class RSHashTableIter VALUE_OBJ_CLASS_SPEC {
   size_t compute_card_ind(CardIdx_t ci);
 
 public:
-  RSHashTableIter() :
-    _tbl_ind(RSHashTable::NullEntry),
+  RSHashTableIter(RSHashTable* rsht) :
+    _tbl_ind(RSHashTable::NullEntry), // So that first increment gets to 0.
     _bl_ind(RSHashTable::NullEntry),
     _card_ind((SparsePRTEntry::cards_num() - 1)),
-    _rsht(NULL) {}
-
-  void init(RSHashTable* rsht) {
-    _rsht = rsht;
-    _tbl_ind = -1; // So that first increment gets to 0.
-    _bl_ind = RSHashTable::NullEntry;
-    _card_ind = (SparsePRTEntry::cards_num() - 1);
-  }
+    _rsht(rsht) {}
 
   bool has_next(size_t& card_index);
 };
@@ -284,8 +277,6 @@ public:
   static void cleanup_all();
   RSHashTable* cur() const { return _cur; }
 
-  void init_iterator(SparsePRTIter* sprt_iter);
-
   static void add_to_expanded_list(SparsePRT* sprt);
   static SparsePRT* get_from_expanded_list();
 
@@ -321,9 +312,9 @@ public:
 
 class SparsePRTIter: public RSHashTableIter {
 public:
-  void init(const SparsePRT* sprt) {
-    RSHashTableIter::init(sprt->cur());
-  }
+  SparsePRTIter(const SparsePRT* sprt) :
+    RSHashTableIter(sprt->cur()) {}
+
   bool has_next(size_t& card_index) {
     return RSHashTableIter::has_next(card_index);
   }