8066102: Clean up HeapRegionRemSet files
authortschatzl
Tue, 09 Dec 2014 12:47:19 +0100
changeset 28030 c36c6703366d
parent 28029 3feb13c88e48
child 28031 10aec24d2b61
8066102: Clean up HeapRegionRemSet files Summary: Remove dead code, tighten public interfaces and improve documentation in the HeapRegionRemSet implementation. Reviewed-by: mgerdin, kbarrett
hotspot/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp
hotspot/src/share/vm/gc_implementation/g1/heapRegionRemSet.cpp
hotspot/src/share/vm/gc_implementation/g1/heapRegionRemSet.hpp
--- a/hotspot/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp	Mon Dec 08 18:52:03 2014 +0100
+++ b/hotspot/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp	Tue Dec 09 12:47:19 2014 +0100
@@ -352,7 +352,7 @@
 }
 
 void G1RegionMappingChangedListener::reset_from_card_cache(uint start_idx, size_t num_regions) {
-  OtherRegionsTable::invalidate(start_idx, num_regions);
+  HeapRegionRemSet::invalidate_from_card_cache(start_idx, num_regions);
 }
 
 void G1RegionMappingChangedListener::on_commit(uint start_idx, size_t num_regions, bool zero_filled) {
--- a/hotspot/src/share/vm/gc_implementation/g1/heapRegionRemSet.cpp	Mon Dec 08 18:52:03 2014 +0100
+++ b/hotspot/src/share/vm/gc_implementation/g1/heapRegionRemSet.cpp	Tue Dec 09 12:47:19 2014 +0100
@@ -407,20 +407,8 @@
   }
 }
 
-void OtherRegionsTable::initialize(uint max_regions) {
-  FromCardCache::initialize(HeapRegionRemSet::num_par_rem_sets(), max_regions);
-}
-
-void OtherRegionsTable::invalidate(uint start_idx, size_t num_regions) {
-  FromCardCache::invalidate(start_idx, num_regions);
-}
-
-void OtherRegionsTable::print_from_card_cache() {
-  FromCardCache::print();
-}
-
 void OtherRegionsTable::add_reference(OopOrNarrowOopStar from, uint tid) {
-  uint cur_hrm_ind = hr()->hrm_index();
+  uint cur_hrm_ind = _hr->hrm_index();
 
   if (G1TraceHeapRegionRememberedSet) {
     gclog_or_tty->print_cr("ORT::add_reference_work(" PTR_FORMAT "->" PTR_FORMAT ").",
@@ -434,7 +422,7 @@
 
   if (G1TraceHeapRegionRememberedSet) {
     gclog_or_tty->print_cr("Table for [" PTR_FORMAT "...): card %d (cache = %d)",
-                  hr()->bottom(), from_card,
+                  _hr->bottom(), from_card,
                   FromCardCache::at(tid, cur_hrm_ind));
   }
 
@@ -477,13 +465,13 @@
       if (G1HRRSUseSparseTable &&
           _sparse_table.add_card(from_hrm_ind, card_index)) {
         if (G1RecordHRRSOops) {
-          HeapRegionRemSet::record(hr(), from);
+          HeapRegionRemSet::record(_hr, from);
           if (G1TraceHeapRegionRememberedSet) {
             gclog_or_tty->print("   Added card " PTR_FORMAT " to region "
                                 "[" PTR_FORMAT "...) for ref " PTR_FORMAT ".\n",
                                 align_size_down(uintptr_t(from),
                                                 CardTableModRefBS::card_size),
-                                hr()->bottom(), from);
+                                _hr->bottom(), from);
           }
         }
         if (G1TraceHeapRegionRememberedSet) {
@@ -539,13 +527,13 @@
   prt->add_reference(from);
 
   if (G1RecordHRRSOops) {
-    HeapRegionRemSet::record(hr(), from);
+    HeapRegionRemSet::record(_hr, from);
     if (G1TraceHeapRegionRememberedSet) {
       gclog_or_tty->print("Added card " PTR_FORMAT " to region "
                           "[" PTR_FORMAT "...) for ref " PTR_FORMAT ".\n",
                           align_size_down(uintptr_t(from),
                                           CardTableModRefBS::card_size),
-                          hr()->bottom(), from);
+                          _hr->bottom(), from);
     }
   }
   assert(contains_reference(from), "We just added it!");
@@ -614,7 +602,7 @@
     if (G1TraceHeapRegionRememberedSet) {
       gclog_or_tty->print("Coarsened entry in region [" PTR_FORMAT "...] "
                  "for region [" PTR_FORMAT "...] (" SIZE_FORMAT " coarse entries).\n",
-                 hr()->bottom(),
+                 _hr->bottom(),
                  max->hr()->bottom(),
                  _n_coarse_entries);
     }
@@ -627,13 +615,11 @@
   return max;
 }
 
-
-// At present, this must be called stop-world single-threaded.
 void OtherRegionsTable::scrub(CardTableModRefBS* ctbs,
                               BitMap* region_bm, BitMap* card_bm) {
   // First eliminated garbage regions from the coarse map.
   if (G1RSScrubVerbose) {
-    gclog_or_tty->print_cr("Scrubbing region %u:", hr()->hrm_index());
+    gclog_or_tty->print_cr("Scrubbing region %u:", _hr->hrm_index());
   }
 
   assert(_coarse_map.size() == region_bm->size(), "Precondition");
@@ -752,7 +738,7 @@
 }
 
 void OtherRegionsTable::clear_fcc() {
-  FromCardCache::clear(hr()->hrm_index());
+  FromCardCache::clear(_hr->hrm_index());
 }
 
 void OtherRegionsTable::clear() {
@@ -774,27 +760,6 @@
   clear_fcc();
 }
 
-bool OtherRegionsTable::del_single_region_table(size_t ind,
-                                                HeapRegion* hr) {
-  assert(0 <= ind && ind < _max_fine_entries, "Preconditions.");
-  PerRegionTable** prev_addr = &_fine_grain_regions[ind];
-  PerRegionTable* prt = *prev_addr;
-  while (prt != NULL && prt->hr() != hr) {
-    prev_addr = prt->collision_list_next_addr();
-    prt = prt->collision_list_next();
-  }
-  if (prt != NULL) {
-    assert(prt->hr() == hr, "Loop postcondition.");
-    *prev_addr = prt->collision_list_next();
-    unlink_from_all(prt);
-    PerRegionTable::free(prt);
-    _n_fine_entries--;
-    return true;
-  } else {
-    return false;
-  }
-}
-
 bool OtherRegionsTable::contains_reference(OopOrNarrowOopStar from) const {
   // Cast away const in this case.
   MutexLockerEx x((Mutex*)_m, Mutex::_no_safepoint_check_flag);
@@ -975,7 +940,7 @@
   _hrrs(hrrs),
   _g1h(G1CollectedHeap::heap()),
   _coarse_map(&hrrs->_other_regions._coarse_map),
-  _bosa(hrrs->bosa()),
+  _bosa(hrrs->_bosa),
   _is(Sparse),
   // Set these values so that we increment to the first region.
   _coarse_cur_region_index(-1),
--- a/hotspot/src/share/vm/gc_implementation/g1/heapRegionRemSet.hpp	Mon Dec 08 18:52:03 2014 +0100
+++ b/hotspot/src/share/vm/gc_implementation/g1/heapRegionRemSet.hpp	Tue Dec 09 12:47:19 2014 +0100
@@ -162,32 +162,36 @@
   // to hold _m, and the fine-grain table to be full.
   PerRegionTable* delete_region_table();
 
-  // If a PRT for "hr" is in the bucket list indicated by "ind" (which must
-  // be the correct index for "hr"), delete it and return true; else return
-  // false.
-  bool del_single_region_table(size_t ind, HeapRegion* hr);
-
   // link/add the given fine grain remembered set into the "all" list
   void link_to_all(PerRegionTable * prt);
   // unlink/remove the given fine grain remembered set into the "all" list
   void unlink_from_all(PerRegionTable * prt);
 
+  bool contains_reference_locked(OopOrNarrowOopStar from) const;
+
+  // Clear the from_card_cache entries for this region.
+  void clear_fcc();
 public:
+  // Create a new remembered set for the given heap region. The given mutex should
+  // be used to ensure consistency.
   OtherRegionsTable(HeapRegion* hr, Mutex* m);
 
-  HeapRegion* hr() const { return _hr; }
-
   // For now.  Could "expand" some tables in the future, so that this made
   // sense.
   void add_reference(OopOrNarrowOopStar from, uint tid);
 
+  // Returns whether the remembered set contains the given reference.
+  bool contains_reference(OopOrNarrowOopStar from) const;
+
   // Removes any entries shown by the given bitmaps to contain only dead
-  // objects.
+  // objects. Not thread safe.
+  // Set bits in the bitmaps indicate that the given region or card is live.
   void scrub(CardTableModRefBS* ctbs, BitMap* region_bm, BitMap* card_bm);
 
-  // Returns whether this remembered set (and all sub-sets) contain no entries.
+  // Returns whether this remembered set (and all sub-sets) does not contain any entry.
   bool is_empty() const;
 
+  // Returns the number of cards contained in this remembered set.
   size_t occupied() const;
   size_t occ_fine() const;
   size_t occ_coarse() const;
@@ -195,31 +199,17 @@
 
   static jint n_coarsenings() { return _n_coarsenings; }
 
-  // Returns size in bytes.
-  // Not const because it takes a lock.
+  // Returns size of the actual remembered set containers in bytes.
   size_t mem_size() const;
+  // Returns the size of static data in bytes.
   static size_t static_mem_size();
+  // Returns the size of the free list content in bytes.
   static size_t fl_mem_size();
 
-  bool contains_reference(OopOrNarrowOopStar from) const;
-  bool contains_reference_locked(OopOrNarrowOopStar from) const;
-
+  // Clear the entire contents of this remembered set.
   void clear();
 
-  // Specifically clear the from_card_cache.
-  void clear_fcc();
-
   void do_cleanup_work(HRRSCleanupTask* hrrs_cleanup_task);
-
-  // Declare the heap size (in # of regions) to the OtherRegionsTable.
-  // (Uses it to initialize from_card_cache).
-  static void initialize(uint max_regions);
-
-  // Declares that regions between start_idx <= i < start_idx + num_regions are
-  // not in use. Make sure that any entries for these regions are invalid.
-  static void invalidate(uint start_idx, size_t num_regions);
-
-  static void print_from_card_cache();
 };
 
 class HeapRegionRemSet : public CHeapObj<mtGC> {
@@ -233,7 +223,6 @@
 
 private:
   G1BlockOffsetSharedArray* _bosa;
-  G1BlockOffsetSharedArray* bosa() const { return _bosa; }
 
   // A set of code blobs (nmethods) whose code contains pointers into
   // the region that owns this RSet.
@@ -268,10 +257,6 @@
   static uint num_par_rem_sets();
   static void setup_remset_size();
 
-  HeapRegion* hr() const {
-    return _other_regions.hr();
-  }
-
   bool is_empty() const {
     return (strong_code_roots_list_length() == 0) && _other_regions.is_empty();
   }
@@ -305,8 +290,9 @@
     _other_regions.add_reference(from, tid);
   }
 
-  // Removes any entries shown by the given bitmaps to contain only dead
-  // objects.
+  // Removes any entries in the remembered set shown by the given bitmaps to
+  // contain only dead objects. Not thread safe.
+  // One bits in the bitmaps indicate that the given region or card is live.
   void scrub(CardTableModRefBS* ctbs, BitMap* region_bm, BitMap* card_bm);
 
   // The region is being reclaimed; clear its remset, and any mention of
@@ -397,16 +383,16 @@
   // Declare the heap size (in # of regions) to the HeapRegionRemSet(s).
   // (Uses it to initialize from_card_cache).
   static void init_heap(uint max_regions) {
-    OtherRegionsTable::initialize(max_regions);
+    FromCardCache::initialize(num_par_rem_sets(), max_regions);
   }
 
-  static void invalidate(uint start_idx, uint num_regions) {
-    OtherRegionsTable::invalidate(start_idx, num_regions);
+  static void invalidate_from_card_cache(uint start_idx, size_t num_regions) {
+    FromCardCache::invalidate(start_idx, num_regions);
   }
 
 #ifndef PRODUCT
   static void print_from_card_cache() {
-    OtherRegionsTable::print_from_card_cache();
+    FromCardCache::print();
   }
 #endif