8034079: G1: Refactor the HeapRegionSet hierarchy

Reviewed-by: tschatzl, pliden

agent/src/share/classes/sun/jvm/hotspot/gc_implementation/g1/G1CollectedHeap.java

@@ -51,9 +51,9 @@ public class G1CollectedHeap extends SharedHeap {
     static private CIntegerField summaryBytesUsedField;
     // G1MonitoringSupport* _g1mm;
     static private AddressField g1mmField;
-    // MasterOldRegionSet _old_set;
+    // HeapRegionSet _old_set;
     static private long oldSetFieldOffset;
-    // MasterHumongousRegionSet _humongous_set;
+    // HeapRegionSet _humongous_set;
     static private long humongousSetFieldOffset;
 
     static {

agent/src/share/classes/sun/jvm/hotspot/gc_implementation/g1/HeapRegionSetBase.java

@@ -40,12 +40,8 @@ import sun.jvm.hotspot.types.TypeDataBase;
 // Mirror class for HeapRegionSetBase. Represents a group of regions.
 
 public class HeapRegionSetBase extends VMObject {
-    // uint _length;
-    static private CIntegerField lengthField;
-    // uint _region_num;
-    static private CIntegerField regionNumField;
-    // size_t _total_used_bytes;
-    static private CIntegerField totalUsedBytesField;
+
+    static private long countField;
 
     static {
         VM.registerVMInitializedObserver(new Observer() {
@@ -58,21 +54,13 @@ public class HeapRegionSetBase extends VMObject {
     static private synchronized void initialize(TypeDataBase db) {
         Type type = db.lookupType("HeapRegionSetBase");
 
-        lengthField         = type.getCIntegerField("_length");
-        regionNumField      = type.getCIntegerField("_region_num");
-        totalUsedBytesField = type.getCIntegerField("_total_used_bytes");
-    }
-
-    public long length() {
-        return lengthField.getValue(addr);
+        countField = type.getField("_count").getOffset();
     }
 
-    public long regionNum() {
-        return regionNumField.getValue(addr);
-    }
 
-    public long totalUsedBytes() {
-        return totalUsedBytesField.getValue(addr);
+    public HeapRegionSetCount count() {
+        Address countFieldAddr = addr.addOffsetTo(countField);
+        return (HeapRegionSetCount) VMObjectFactory.newObject(HeapRegionSetCount.class, countFieldAddr);
     }
 
     public HeapRegionSetBase(Address addr) {

agent/src/share/classes/sun/jvm/hotspot/gc_implementation/g1/HeapRegionSetCount.java

+/*
+ * Copyright (c) 2014, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ *
+ */
+
+package sun.jvm.hotspot.gc_implementation.g1;
+
+import java.util.Iterator;
+import java.util.Observable;
+import java.util.Observer;
+
+import sun.jvm.hotspot.debugger.Address;
+import sun.jvm.hotspot.runtime.VM;
+import sun.jvm.hotspot.runtime.VMObject;
+import sun.jvm.hotspot.runtime.VMObjectFactory;
+import sun.jvm.hotspot.types.AddressField;
+import sun.jvm.hotspot.types.CIntegerField;
+import sun.jvm.hotspot.types.Type;
+import sun.jvm.hotspot.types.TypeDataBase;
+
+// Mirror class for HeapRegionSetCount. Represents a group of regions.
+
+public class HeapRegionSetCount extends VMObject {
+
+    static private CIntegerField lengthField;
+    static private CIntegerField capacityField;
+
+    static {
+        VM.registerVMInitializedObserver(new Observer() {
+                public void update(Observable o, Object data) {
+                    initialize(VM.getVM().getTypeDataBase());
+                }
+            });
+    }
+
+    static private synchronized void initialize(TypeDataBase db) {
+        Type type = db.lookupType("HeapRegionSetCount");
+
+        lengthField   = type.getCIntegerField("_length");
+        capacityField = type.getCIntegerField("_capacity");
+    }
+
+    public long length() {
+        return lengthField.getValue(addr);
+    }
+
+    public long capacity() {
+        return capacityField.getValue(addr);
+    }
+
+    public HeapRegionSetCount(Address addr) {
+        super(addr);
+    }
+}

agent/src/share/classes/sun/jvm/hotspot/tools/HeapSummary.java

@@ -114,7 +114,8 @@ public class HeapSummary extends Tool {
              long survivorRegionNum = g1mm.survivorRegionNum();
              HeapRegionSetBase oldSet = g1h.oldSet();
              HeapRegionSetBase humongousSet = g1h.humongousSet();
-             long oldRegionNum = oldSet.regionNum() + humongousSet.regionNum();
+             long oldRegionNum = oldSet.count().length()
+                          + humongousSet.count().capacity() / HeapRegion.grainBytes();
              printG1Space("G1 Heap:", g1h.n_regions(),
                           g1h.used(), g1h.capacity());
              System.out.println("G1 Young Generation:");

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

@@ -1809,8 +1809,8 @@ class G1NoteEndOfConcMarkClosure : public HeapRegionClosure {
   uint _regions_claimed;
   size_t _freed_bytes;
   FreeRegionList* _local_cleanup_list;
-  OldRegionSet* _old_proxy_set;
-  HumongousRegionSet* _humongous_proxy_set;
+  HeapRegionSetCount _old_regions_removed;
+  HeapRegionSetCount _humongous_regions_removed;
   HRRSCleanupTask* _hrrs_cleanup_task;
   double _claimed_region_time;
   double _max_region_time;
@@ -1819,19 +1819,19 @@ public:
   G1NoteEndOfConcMarkClosure(G1CollectedHeap* g1,
                              int worker_num,
                              FreeRegionList* local_cleanup_list,
-                             OldRegionSet* old_proxy_set,
-                             HumongousRegionSet* humongous_proxy_set,
                              HRRSCleanupTask* hrrs_cleanup_task) :
     _g1(g1), _worker_num(worker_num),
     _max_live_bytes(0), _regions_claimed(0),
     _freed_bytes(0),
     _claimed_region_time(0.0), _max_region_time(0.0),
     _local_cleanup_list(local_cleanup_list),
-    _old_proxy_set(old_proxy_set),
-    _humongous_proxy_set(humongous_proxy_set),
+    _old_regions_removed(),
+    _humongous_regions_removed(),
     _hrrs_cleanup_task(hrrs_cleanup_task) { }
 
   size_t freed_bytes() { return _freed_bytes; }
+  const HeapRegionSetCount& old_regions_removed() { return _old_regions_removed; }
+  const HeapRegionSetCount& humongous_regions_removed() { return _humongous_regions_removed; }
 
   bool doHeapRegion(HeapRegion *hr) {
     if (hr->continuesHumongous()) {
@@ -1844,13 +1844,22 @@ public:
     _regions_claimed++;
     hr->note_end_of_marking();
     _max_live_bytes += hr->max_live_bytes();
-    _g1->free_region_if_empty(hr,
-                              &_freed_bytes,
-                              _local_cleanup_list,
-                              _old_proxy_set,
-                              _humongous_proxy_set,
-                              _hrrs_cleanup_task,
-                              true /* par */);
+
+    if (hr->used() > 0 && hr->max_live_bytes() == 0 && !hr->is_young()) {
+      _freed_bytes += hr->used();
+      hr->set_containing_set(NULL);
+      if (hr->isHumongous()) {
+        assert(hr->startsHumongous(), "we should only see starts humongous");
+        _humongous_regions_removed.increment(1u, hr->capacity());
+        _g1->free_humongous_region(hr, _local_cleanup_list, true);
+      } else {
+        _old_regions_removed.increment(1u, hr->capacity());
+        _g1->free_region(hr, _local_cleanup_list, true);
+      }
+    } else {
+      hr->rem_set()->do_cleanup_work(_hrrs_cleanup_task);
+    }
+
     double region_time = (os::elapsedTime() - start);
     _claimed_region_time += region_time;
     if (region_time > _max_region_time) {
@@ -1883,12 +1892,8 @@ public:
   void work(uint worker_id) {
     double start = os::elapsedTime();
     FreeRegionList local_cleanup_list("Local Cleanup List");
-    OldRegionSet old_proxy_set("Local Cleanup Old Proxy Set");
-    HumongousRegionSet humongous_proxy_set("Local Cleanup Humongous Proxy Set");
     HRRSCleanupTask hrrs_cleanup_task;
     G1NoteEndOfConcMarkClosure g1_note_end(_g1h, worker_id, &local_cleanup_list,
-                                           &old_proxy_set,
-                                           &humongous_proxy_set,
                                            &hrrs_cleanup_task);
     if (G1CollectedHeap::use_parallel_gc_threads()) {
       _g1h->heap_region_par_iterate_chunked(&g1_note_end, worker_id,
@@ -1900,13 +1905,10 @@ public:
     assert(g1_note_end.complete(), "Shouldn't have yielded!");
 
     // Now update the lists
-    _g1h->update_sets_after_freeing_regions(g1_note_end.freed_bytes(),
-                                            NULL /* free_list */,
-                                            &old_proxy_set,
-                                            &humongous_proxy_set,
-                                            true /* par */);
+    _g1h->remove_from_old_sets(g1_note_end.old_regions_removed(), g1_note_end.humongous_regions_removed());
     {
       MutexLockerEx x(ParGCRareEvent_lock, Mutex::_no_safepoint_check_flag);
+      _g1h->decrement_summary_bytes(g1_note_end.freed_bytes());
       _max_live_bytes += g1_note_end.max_live_bytes();
       _freed_bytes += g1_note_end.freed_bytes();
 
@@ -1920,7 +1922,7 @@ public:
 
       G1HRPrinter* hr_printer = _g1h->hr_printer();
       if (hr_printer->is_active()) {
-        HeapRegionLinkedListIterator iter(&local_cleanup_list);
+        FreeRegionListIterator iter(&local_cleanup_list);
         while (iter.more_available()) {
           HeapRegion* hr = iter.get_next();
           hr_printer->cleanup(hr);
@@ -1971,7 +1973,6 @@ void ConcurrentMark::cleanup() {
     return;
   }
 
-  HRSPhaseSetter x(HRSPhaseCleanup);
   g1h->verify_region_sets_optional();
 
   if (VerifyDuringGC) {
@@ -2144,7 +2145,7 @@ void ConcurrentMark::completeCleanup() {
 
   G1CollectedHeap* g1h = G1CollectedHeap::heap();
 
-  _cleanup_list.verify_optional();
+  _cleanup_list.verify_list();
   FreeRegionList tmp_free_list("Tmp Free List");
 
   if (G1ConcRegionFreeingVerbose) {

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

@@ -25,7 +25,7 @@
 #ifndef SHARE_VM_GC_IMPLEMENTATION_G1_CONCURRENTMARK_HPP
 #define SHARE_VM_GC_IMPLEMENTATION_G1_CONCURRENTMARK_HPP
 
-#include "gc_implementation/g1/heapRegionSets.hpp"
+#include "gc_implementation/g1/heapRegionSet.hpp"
 #include "utilities/taskqueue.hpp"
 
 class G1CollectedHeap;

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

@@ -1298,7 +1298,6 @@ bool G1CollectedHeap::do_collection(bool explicit_gc,
 
   size_t metadata_prev_used = MetaspaceAux::allocated_used_bytes();
 
-  HRSPhaseSetter x(HRSPhaseFullGC);
   verify_region_sets_optional();
 
   const bool do_clear_all_soft_refs = clear_all_soft_refs ||
@@ -1928,10 +1927,10 @@ G1CollectedHeap::G1CollectedHeap(G1CollectorPolicy* policy_) :
   _g1mm(NULL),
   _refine_cte_cl(NULL),
   _full_collection(false),
-  _free_list("Master Free List"),
-  _secondary_free_list("Secondary Free List"),
-  _old_set("Old Set"),
-  _humongous_set("Master Humongous Set"),
+  _free_list("Master Free List", new MasterFreeRegionListMtSafeChecker()),
+  _secondary_free_list("Secondary Free List", new SecondaryFreeRegionListMtSafeChecker()),
+  _old_set("Old Set", false /* humongous */, new OldRegionSetMtSafeChecker()),
+  _humongous_set("Master Humongous Set", true /* humongous */, new HumongousRegionSetMtSafeChecker()),
   _free_regions_coming(false),
   _young_list(new YoungList(this)),
   _gc_time_stamp(0),
@@ -2079,7 +2078,7 @@ jint G1CollectedHeap::initialize() {
   guarantee(HeapRegion::CardsPerRegion < max_cards_per_region,
             "too many cards per region");
 
-  HeapRegionSet::set_unrealistically_long_length(max_regions() + 1);
+  FreeRegionList::set_unrealistically_long_length(max_regions() + 1);
 
   _bot_shared = new G1BlockOffsetSharedArray(_reserved,
                                              heap_word_size(init_byte_size));
@@ -3905,7 +3904,6 @@ G1CollectedHeap::do_collection_pause_at_safepoint(double target_pause_time_ms) {
   print_heap_before_gc();
   trace_heap_before_gc(_gc_tracer_stw);
 
-  HRSPhaseSetter x(HRSPhaseEvacuation);
   verify_region_sets_optional();
   verify_dirty_young_regions();
 
@@ -5967,28 +5965,7 @@ void G1CollectedHeap::evacuate_collection_set(EvacuationInfo& evacuation_info) {
   COMPILER2_PRESENT(DerivedPointerTable::update_pointers());
 }
 
-void G1CollectedHeap::free_region_if_empty(HeapRegion* hr,
-                                     size_t* pre_used,
-                                     FreeRegionList* free_list,
-                                     OldRegionSet* old_proxy_set,
-                                     HumongousRegionSet* humongous_proxy_set,
-                                     HRRSCleanupTask* hrrs_cleanup_task,
-                                     bool par) {
-  if (hr->used() > 0 && hr->max_live_bytes() == 0 && !hr->is_young()) {
-    if (hr->isHumongous()) {
-      assert(hr->startsHumongous(), "we should only see starts humongous");
-      free_humongous_region(hr, pre_used, free_list, humongous_proxy_set, par);
-    } else {
-      _old_set.remove_with_proxy(hr, old_proxy_set);
-      free_region(hr, pre_used, free_list, par);
-    }
-  } else {
-    hr->rem_set()->do_cleanup_work(hrrs_cleanup_task);
-  }
-}
-
 void G1CollectedHeap::free_region(HeapRegion* hr,
-                                  size_t* pre_used,
                                   FreeRegionList* free_list,
                                   bool par) {
   assert(!hr->isHumongous(), "this is only for non-humongous regions");
@@ -6001,72 +5978,58 @@ void G1CollectedHeap::free_region(HeapRegion* hr,
   if (!hr->is_young()) {
     _cg1r->hot_card_cache()->reset_card_counts(hr);
   }
-  *pre_used += hr->used();
   hr->hr_clear(par, true /* clear_space */);
   free_list->add_as_head(hr);
 }
 
 void G1CollectedHeap::free_humongous_region(HeapRegion* hr,
-                                     size_t* pre_used,
                                      FreeRegionList* free_list,
-                                     HumongousRegionSet* humongous_proxy_set,
                                      bool par) {
   assert(hr->startsHumongous(), "this is only for starts humongous regions");
   assert(free_list != NULL, "pre-condition");
-  assert(humongous_proxy_set != NULL, "pre-condition");
 
-  size_t hr_used = hr->used();
   size_t hr_capacity = hr->capacity();
-  size_t hr_pre_used = 0;
-  _humongous_set.remove_with_proxy(hr, humongous_proxy_set);
   // We need to read this before we make the region non-humongous,
   // otherwise the information will be gone.
   uint last_index = hr->last_hc_index();
   hr->set_notHumongous();
-  free_region(hr, &hr_pre_used, free_list, par);
+  free_region(hr, free_list, par);
 
   uint i = hr->hrs_index() + 1;
   while (i < last_index) {
     HeapRegion* curr_hr = region_at(i);
     assert(curr_hr->continuesHumongous(), "invariant");
     curr_hr->set_notHumongous();
-    free_region(curr_hr, &hr_pre_used, free_list, par);
+    free_region(curr_hr, free_list, par);
     i += 1;
   }
-  assert(hr_pre_used == hr_used,
-         err_msg("hr_pre_used: "SIZE_FORMAT" and hr_used: "SIZE_FORMAT" "
-                 "should be the same", hr_pre_used, hr_used));
-  *pre_used += hr_pre_used;
-}
-
-void G1CollectedHeap::update_sets_after_freeing_regions(size_t pre_used,
-                                       FreeRegionList* free_list,
-                                       OldRegionSet* old_proxy_set,
-                                       HumongousRegionSet* humongous_proxy_set,
-                                       bool par) {
-  if (pre_used > 0) {
-    Mutex* lock = (par) ? ParGCRareEvent_lock : NULL;
-    MutexLockerEx x(lock, Mutex::_no_safepoint_check_flag);
-    assert(_summary_bytes_used >= pre_used,
-           err_msg("invariant: _summary_bytes_used: "SIZE_FORMAT" "
-                   "should be >= pre_used: "SIZE_FORMAT,
-                   _summary_bytes_used, pre_used));
-    _summary_bytes_used -= pre_used;
-  }
-  if (free_list != NULL && !free_list->is_empty()) {
-    MutexLockerEx x(FreeList_lock, Mutex::_no_safepoint_check_flag);
-    _free_list.add_as_head(free_list);
-  }
-  if (old_proxy_set != NULL && !old_proxy_set->is_empty()) {
+}
+
+void G1CollectedHeap::remove_from_old_sets(const HeapRegionSetCount& old_regions_removed,
+                                       const HeapRegionSetCount& humongous_regions_removed) {
+  if (old_regions_removed.length() > 0 || humongous_regions_removed.length() > 0) {
     MutexLockerEx x(OldSets_lock, Mutex::_no_safepoint_check_flag);
-    _old_set.update_from_proxy(old_proxy_set);
+    _old_set.bulk_remove(old_regions_removed);
+    _humongous_set.bulk_remove(humongous_regions_removed);
   }
-  if (humongous_proxy_set != NULL && !humongous_proxy_set->is_empty()) {
-    MutexLockerEx x(OldSets_lock, Mutex::_no_safepoint_check_flag);
-    _humongous_set.update_from_proxy(humongous_proxy_set);
+
+}
+
+void G1CollectedHeap::prepend_to_freelist(FreeRegionList* list) {
+  assert(list != NULL, "list can't be null");
+  if (!list->is_empty()) {
+    MutexLockerEx x(FreeList_lock, Mutex::_no_safepoint_check_flag);
+    _free_list.add_as_head(list);
   }
 }
 
+void G1CollectedHeap::decrement_summary_bytes(size_t bytes) {
+  assert(_summary_bytes_used >= bytes,
+         err_msg("invariant: _summary_bytes_used: "SIZE_FORMAT" should be >= bytes: "SIZE_FORMAT,
+                  _summary_bytes_used, bytes));
+  _summary_bytes_used -= bytes;
+}
+
 class G1ParCleanupCTTask : public AbstractGangTask {
   G1SATBCardTableModRefBS* _ct_bs;
   G1CollectedHeap* _g1h;
@@ -6257,7 +6220,8 @@ void G1CollectedHeap::free_collection_set(HeapRegion* cs_head, EvacuationInfo& e
 
       // And the region is empty.
       assert(!used_mr.is_empty(), "Should not have empty regions in a CS.");
-      free_region(cur, &pre_used, &local_free_list, false /* par */);
+      pre_used += cur->used();
+      free_region(cur, &local_free_list, false /* par */);
     } else {
       cur->uninstall_surv_rate_group();
       if (cur->is_young()) {
@@ -6285,10 +6249,8 @@ void G1CollectedHeap::free_collection_set(HeapRegion* cs_head, EvacuationInfo& e
     young_time_ms += elapsed_ms;
   }
 
-  update_sets_after_freeing_regions(pre_used, &local_free_list,
-                                    NULL /* old_proxy_set */,
-                                    NULL /* humongous_proxy_set */,
-                                    false /* par */);
+  prepend_to_freelist(&local_free_list);
+  decrement_summary_bytes(pre_used);
   policy->phase_times()->record_young_free_cset_time_ms(young_time_ms);
   policy->phase_times()->record_non_young_free_cset_time_ms(non_young_time_ms);
 }
@@ -6400,10 +6362,10 @@ bool G1CollectedHeap::check_young_list_empty(bool check_heap, bool check_sample)
 
 class TearDownRegionSetsClosure : public HeapRegionClosure {
 private:
-  OldRegionSet *_old_set;
+  HeapRegionSet *_old_set;
 
 public:
-  TearDownRegionSetsClosure(OldRegionSet* old_set) : _old_set(old_set) { }
+  TearDownRegionSetsClosure(HeapRegionSet* old_set) : _old_set(old_set) { }
 
   bool doHeapRegion(HeapRegion* r) {
     if (r->is_empty()) {
@@ -6442,13 +6404,13 @@ void G1CollectedHeap::tear_down_region_sets(bool free_list_only) {
 class RebuildRegionSetsClosure : public HeapRegionClosure {
 private:
   bool            _free_list_only;
-  OldRegionSet*   _old_set;
+  HeapRegionSet*   _old_set;
   FreeRegionList* _free_list;
   size_t          _total_used;
 
 public:
   RebuildRegionSetsClosure(bool free_list_only,
-                           OldRegionSet* old_set, FreeRegionList* free_list) :
+                           HeapRegionSet* old_set, FreeRegionList* free_list) :
     _free_list_only(free_list_only),
     _old_set(old_set), _free_list(free_list), _total_used(0) {
     assert(_free_list->is_empty(), "pre-condition");
@@ -6645,23 +6607,22 @@ void OldGCAllocRegion::retire_region(HeapRegion* alloc_region,
 
 class VerifyRegionListsClosure : public HeapRegionClosure {
 private:
-  FreeRegionList*     _free_list;
-  OldRegionSet*       _old_set;
-  HumongousRegionSet* _humongous_set;
-  uint                _region_count;
+  HeapRegionSet*   _old_set;
+  HeapRegionSet*   _humongous_set;
+  FreeRegionList*  _free_list;
 
 public:
-  VerifyRegionListsClosure(OldRegionSet* old_set,
-                           HumongousRegionSet* humongous_set,
-                           FreeRegionList* free_list) :
-    _old_set(old_set), _humongous_set(humongous_set),
-    _free_list(free_list), _region_count(0) { }
+  HeapRegionSetCount _old_count;
+  HeapRegionSetCount _humongous_count;
+  HeapRegionSetCount _free_count;
 
-  uint region_count() { return _region_count; }
+  VerifyRegionListsClosure(HeapRegionSet* old_set,
+                           HeapRegionSet* humongous_set,
+                           FreeRegionList* free_list) :
+    _old_set(old_set), _humongous_set(humongous_set), _free_list(free_list),
+    _old_count(), _humongous_count(), _free_count(){ }
 
   bool doHeapRegion(HeapRegion* hr) {
-    _region_count += 1;
-
     if (hr->continuesHumongous()) {
       return false;
     }
@@ -6669,14 +6630,31 @@ public:
     if (hr->is_young()) {
       // TODO
     } else if (hr->startsHumongous()) {
-      _humongous_set->verify_next_region(hr);
+      assert(hr->containing_set() == _humongous_set, err_msg("Heap region %u is starts humongous but not in humongous set.", hr->region_num()));
+      _humongous_count.increment(1u, hr->capacity());
     } else if (hr->is_empty()) {
-      _free_list->verify_next_region(hr);
+      assert(hr->containing_set() == _free_list, err_msg("Heap region %u is empty but not on the free list.", hr->region_num()));
+      _free_count.increment(1u, hr->capacity());
     } else {
-      _old_set->verify_next_region(hr);
+      assert(hr->containing_set() == _old_set, err_msg("Heap region %u is old but not in the old set.", hr->region_num()));
+      _old_count.increment(1u, hr->capacity());
     }
     return false;
   }
+
+  void verify_counts(HeapRegionSet* old_set, HeapRegionSet* humongous_set, FreeRegionList* free_list) {
+    guarantee(old_set->length() == _old_count.length(), err_msg("Old set count mismatch. Expected %u, actual %u.", old_set->length(), _old_count.length()));
+    guarantee(old_set->total_capacity_bytes() == _old_count.capacity(), err_msg("Old set capacity mismatch. Expected " SIZE_FORMAT ", actual " SIZE_FORMAT,
+        old_set->total_capacity_bytes(), _old_count.capacity()));
+
+    guarantee(humongous_set->length() == _humongous_count.length(), err_msg("Hum set count mismatch. Expected %u, actual %u.", humongous_set->length(), _humongous_count.length()));
+    guarantee(humongous_set->total_capacity_bytes() == _humongous_count.capacity(), err_msg("Hum set capacity mismatch. Expected " SIZE_FORMAT ", actual " SIZE_FORMAT,
+        humongous_set->total_capacity_bytes(), _humongous_count.capacity()));
+
+    guarantee(free_list->length() == _free_count.length(), err_msg("Free list count mismatch. Expected %u, actual %u.", free_list->length(), _free_count.length()));
+    guarantee(free_list->total_capacity_bytes() == _free_count.capacity(), err_msg("Free list capacity mismatch. Expected " SIZE_FORMAT ", actual " SIZE_FORMAT,
+        free_list->total_capacity_bytes(), _free_count.capacity()));
+  }
 };
 
 HeapRegion* G1CollectedHeap::new_heap_region(uint hrs_index,
@@ -6692,16 +6670,14 @@ void G1CollectedHeap::verify_region_sets() {
   assert_heap_locked_or_at_safepoint(true /* should_be_vm_thread */);
 
   // First, check the explicit lists.
-  _free_list.verify();
+  _free_list.verify_list();
   {
     // Given that a concurrent operation might be adding regions to
     // the secondary free list we have to take the lock before
     // verifying it.
     MutexLockerEx x(SecondaryFreeList_lock, Mutex::_no_safepoint_check_flag);
-    _secondary_free_list.verify();
+    _secondary_free_list.verify_list();
   }
-  _old_set.verify();
-  _humongous_set.verify();
 
   // If a concurrent region freeing operation is in progress it will
   // be difficult to correctly attributed any free regions we come
@@ -6724,16 +6700,10 @@ void G1CollectedHeap::verify_region_sets() {
 
   // Finally, make sure that the region accounting in the lists is
   // consistent with what we see in the heap.
-  _old_set.verify_start();
-  _humongous_set.verify_start();
-  _free_list.verify_start();
 
   VerifyRegionListsClosure cl(&_old_set, &_humongous_set, &_free_list);
   heap_region_iterate(&cl);
-
-  _old_set.verify_end();
-  _humongous_set.verify_end();
-  _free_list.verify_end();
+  cl.verify_counts(&_old_set, &_humongous_set, &_free_list);
 }
 
 // Optimized nmethod scanning

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

@@ -34,7 +34,7 @@
 #include "gc_implementation/g1/g1SATBCardTableModRefBS.hpp"
 #include "gc_implementation/g1/g1YCTypes.hpp"
 #include "gc_implementation/g1/heapRegionSeq.hpp"
-#include "gc_implementation/g1/heapRegionSets.hpp"
+#include "gc_implementation/g1/heapRegionSet.hpp"
 #include "gc_implementation/shared/hSpaceCounters.hpp"
 #include "gc_implementation/shared/parGCAllocBuffer.hpp"
 #include "memory/barrierSet.hpp"
@@ -243,18 +243,18 @@ private:
   MemRegion _g1_committed;
 
   // The master free list. It will satisfy all new region allocations.
-  MasterFreeRegionList      _free_list;
+  FreeRegionList _free_list;
 
   // The secondary free list which contains regions that have been
   // freed up during the cleanup process. This will be appended to the
   // master free list when appropriate.
-  SecondaryFreeRegionList   _secondary_free_list;
+  FreeRegionList _secondary_free_list;
 
   // It keeps track of the old regions.
-  MasterOldRegionSet        _old_set;
+  HeapRegionSet _old_set;
 
   // It keeps track of the humongous regions.
-  MasterHumongousRegionSet  _humongous_set;
+  HeapRegionSet _humongous_set;
 
   // The number of regions we could create by expansion.
   uint _expansion_regions;
@@ -757,6 +757,26 @@ public:
 
   G1HRPrinter* hr_printer() { return &_hr_printer; }
 
+  // Frees a non-humongous region by initializing its contents and
+  // adding it to the free list that's passed as a parameter (this is
+  // usually a local list which will be appended to the master free
+  // list later). The used bytes of freed regions are accumulated in
+  // pre_used. If par is true, the region's RSet will not be freed
+  // up. The assumption is that this will be done later.
+  void free_region(HeapRegion* hr,
+                   FreeRegionList* free_list,
+                   bool par);
+
+  // Frees a humongous region by collapsing it into individual regions
+  // and calling free_region() for each of them. The freed regions
+  // will be added to the free list that's passed as a parameter (this
+  // is usually a local list which will be appended to the master free
+  // list later). The used bytes of freed regions are accumulated in
+  // pre_used. If par is true, the region's RSet will not be freed
+  // up. The assumption is that this will be done later.
+  void free_humongous_region(HeapRegion* hr,
+                             FreeRegionList* free_list,
+                             bool par);
 protected:
 
   // Shrink the garbage-first heap by at most the given size (in bytes!).
@@ -840,30 +860,6 @@ protected:
   // string table, and referents of reachable weak refs.
   void g1_process_weak_roots(OopClosure* root_closure);
 
-  // Frees a non-humongous region by initializing its contents and
-  // adding it to the free list that's passed as a parameter (this is
-  // usually a local list which will be appended to the master free
-  // list later). The used bytes of freed regions are accumulated in
-  // pre_used. If par is true, the region's RSet will not be freed
-  // up. The assumption is that this will be done later.
-  void free_region(HeapRegion* hr,
-                   size_t* pre_used,
-                   FreeRegionList* free_list,
-                   bool par);
-
-  // Frees a humongous region by collapsing it into individual regions
-  // and calling free_region() for each of them. The freed regions
-  // will be added to the free list that's passed as a parameter (this
-  // is usually a local list which will be appended to the master free
-  // list later). The used bytes of freed regions are accumulated in
-  // pre_used. If par is true, the region's RSet will not be freed
-  // up. The assumption is that this will be done later.
-  void free_humongous_region(HeapRegion* hr,
-                             size_t* pre_used,
-                             FreeRegionList* free_list,
-                             HumongousRegionSet* humongous_proxy_set,
-                             bool par);
-
   // Notifies all the necessary spaces that the committed space has
   // been updated (either expanded or shrunk). It should be called
   // after _g1_storage is updated.
@@ -1242,10 +1238,6 @@ public:
   bool is_on_master_free_list(HeapRegion* hr) {
     return hr->containing_set() == &_free_list;
   }
-
-  bool is_in_humongous_set(HeapRegion* hr) {
-    return hr->containing_set() == &_humongous_set;
-  }
 #endif // ASSERT
 
   // Wrapper for the region list operations that can be called from
@@ -1298,27 +1290,9 @@ public:
   // True iff an evacuation has failed in the most-recent collection.
   bool evacuation_failed() { return _evacuation_failed; }
 
-  // It will free a region if it has allocated objects in it that are
-  // all dead. It calls either free_region() or
-  // free_humongous_region() depending on the type of the region that
-  // is passed to it.
-  void free_region_if_empty(HeapRegion* hr,
-                            size_t* pre_used,
-                            FreeRegionList* free_list,
-                            OldRegionSet* old_proxy_set,
-                            HumongousRegionSet* humongous_proxy_set,
-                            HRRSCleanupTask* hrrs_cleanup_task,
-                            bool par);
-
-  // It appends the free list to the master free list and updates the
-  // master humongous list according to the contents of the proxy
-  // list. It also adjusts the total used bytes according to pre_used
-  // (if par is true, it will do so by taking the ParGCRareEvent_lock).
-  void update_sets_after_freeing_regions(size_t pre_used,
-                                       FreeRegionList* free_list,
-                                       OldRegionSet* old_proxy_set,
-                                       HumongousRegionSet* humongous_proxy_set,
-                                       bool par);
+  void remove_from_old_sets(const HeapRegionSetCount& old_regions_removed, const HeapRegionSetCount& humongous_regions_removed);
+  void prepend_to_freelist(FreeRegionList* list);
+  void decrement_summary_bytes(size_t bytes);
 
   // Returns "TRUE" iff "p" points into the committed areas of the heap.
   virtual bool is_in(const void* p) const;

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

@@ -30,6 +30,7 @@
 #include "gc_implementation/g1/g1AllocRegion.inline.hpp"
 #include "gc_implementation/g1/g1CollectorPolicy.hpp"
 #include "gc_implementation/g1/g1SATBCardTableModRefBS.hpp"
+#include "gc_implementation/g1/heapRegionSet.inline.hpp"
 #include "gc_implementation/g1/heapRegionSeq.inline.hpp"
 #include "utilities/taskqueue.hpp"
 

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

@@ -196,17 +196,19 @@ class G1PrepareCompactClosure: public HeapRegionClosure {
   G1CollectedHeap* _g1h;
   ModRefBarrierSet* _mrbs;
   CompactPoint _cp;
-  HumongousRegionSet _humongous_proxy_set;
+  HeapRegionSetCount _humongous_regions_removed;
 
   void free_humongous_region(HeapRegion* hr) {
     HeapWord* end = hr->end();
-    size_t dummy_pre_used;
     FreeRegionList dummy_free_list("Dummy Free List for G1MarkSweep");
 
     assert(hr->startsHumongous(),
            "Only the start of a humongous region should be freed.");
-    _g1h->free_humongous_region(hr, &dummy_pre_used, &dummy_free_list,
-                                &_humongous_proxy_set, false /* par */);
+
+    hr->set_containing_set(NULL);
+    _humongous_regions_removed.increment(1u, hr->capacity());
+
+    _g1h->free_humongous_region(hr, &dummy_free_list, false /* par */);
     hr->prepare_for_compaction(&_cp);
     // Also clear the part of the card table that will be unused after
     // compaction.
@@ -219,16 +221,13 @@ public:
   : _g1h(G1CollectedHeap::heap()),
     _mrbs(_g1h->g1_barrier_set()),
     _cp(NULL, cs, cs->initialize_threshold()),
-    _humongous_proxy_set("G1MarkSweep Humongous Proxy Set") { }
+    _humongous_regions_removed() { }
 
   void update_sets() {
     // We'll recalculate total used bytes and recreate the free list
     // at the end of the GC, so no point in updating those values here.
-    _g1h->update_sets_after_freeing_regions(0, /* pre_used */
-                                            NULL, /* free_list */
-                                            NULL, /* old_proxy_set */
-                                            &_humongous_proxy_set,
-                                            false /* par */);
+    HeapRegionSetCount empty_set;
+    _g1h->remove_from_old_sets(empty_set, _humongous_regions_removed);
   }
 
   bool doHeapRegion(HeapRegion* hr) {

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

@@ -25,7 +25,7 @@
 #include "precompiled.hpp"
 #include "gc_implementation/g1/heapRegion.hpp"
 #include "gc_implementation/g1/heapRegionSeq.inline.hpp"
-#include "gc_implementation/g1/heapRegionSets.hpp"
+#include "gc_implementation/g1/heapRegionSet.hpp"
 #include "gc_implementation/g1/g1CollectedHeap.inline.hpp"
 #include "memory/allocation.hpp"
 

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

@@ -23,171 +23,60 @@
  */
 
 #include "precompiled.hpp"
+#include "gc_implementation/g1/heapRegionRemSet.hpp"
 #include "gc_implementation/g1/heapRegionSet.inline.hpp"
 
-uint HeapRegionSetBase::_unrealistically_long_length = 0;
-HRSPhase HeapRegionSetBase::_phase = HRSPhaseNone;
-
-//////////////////// HeapRegionSetBase ////////////////////
-
-void HeapRegionSetBase::set_unrealistically_long_length(uint len) {
-  guarantee(_unrealistically_long_length == 0, "should only be set once");
-  _unrealistically_long_length = len;
-}
+uint FreeRegionList::_unrealistically_long_length = 0;
 
 void HeapRegionSetBase::fill_in_ext_msg(hrs_ext_msg* msg, const char* message) {
-  msg->append("[%s] %s ln: %u rn: %u cy: "SIZE_FORMAT" ud: "SIZE_FORMAT,
-              name(), message, length(), region_num(),
-              total_capacity_bytes(), total_used_bytes());
+  msg->append("[%s] %s ln: %u cy: "SIZE_FORMAT,
+              name(), message, length(), total_capacity_bytes());
   fill_in_ext_msg_extra(msg);
 }
 
-bool HeapRegionSetBase::verify_region(HeapRegion* hr,
-                                  HeapRegionSetBase* expected_containing_set) {
-  const char* error_message = NULL;
-
-  if (!regions_humongous()) {
-    if (hr->isHumongous()) {
-      error_message = "the region should not be humongous";
-    }
-  } else {
-    if (!hr->isHumongous() || !hr->startsHumongous()) {
-      error_message = "the region should be 'starts humongous'";
-    }
-  }
-
-  if (!regions_empty()) {
-    if (hr->is_empty()) {
-      error_message = "the region should not be empty";
-    }
-  } else {
-    if (!hr->is_empty()) {
-      error_message = "the region should be empty";
-    }
-  }
-
-#ifdef ASSERT
-  // The _containing_set field is only available when ASSERT is defined.
-  if (hr->containing_set() != expected_containing_set) {
-    error_message = "inconsistent containing set found";
-  }
-#endif // ASSERT
-
-  const char* extra_error_message = verify_region_extra(hr);
-  if (extra_error_message != NULL) {
-    error_message = extra_error_message;
-  }
-
-  if (error_message != NULL) {
-    outputStream* out = tty;
-    out->cr();
-    out->print_cr("## [%s] %s", name(), error_message);
-    out->print_cr("## Offending Region: "PTR_FORMAT, hr);
-    out->print_cr("   "HR_FORMAT, HR_FORMAT_PARAMS(hr));
-#ifdef ASSERT
-    out->print_cr("   containing set: "PTR_FORMAT, hr->containing_set());
-#endif // ASSERT
-    out->print_cr("## Offending Region Set: "PTR_FORMAT, this);
-    print_on(out);
-    return false;
-  } else {
-    return true;
-  }
+#ifndef PRODUCT
+void HeapRegionSetBase::verify_region(HeapRegion* hr) {
+  assert(hr->containing_set() == this, err_msg("Inconsistent containing set for %u", hr->hrs_index()));
+  assert(!hr->is_young(), err_msg("Adding young region %u", hr->hrs_index())); // currently we don't use these sets for young regions
+  assert(hr->isHumongous() == regions_humongous(), err_msg("Wrong humongous state for region %u and set %s", hr->hrs_index(), name()));
+  assert(hr->is_empty() == regions_empty(), err_msg("Wrong empty state for region %u and set %s", hr->hrs_index(), name()));
+  assert(hr->rem_set()->verify_ready_for_par_iteration(), err_msg("Wrong iteration state %u", hr->hrs_index()));
 }
+#endif
 
 void HeapRegionSetBase::verify() {
   // It's important that we also observe the MT safety protocol even
   // for the verification calls. If we do verification without the
   // appropriate locks and the set changes underneath our feet
   // verification might fail and send us on a wild goose chase.
-  hrs_assert_mt_safety_ok(this);
-
-  guarantee(( is_empty() && length() == 0 && region_num() == 0 &&
-              total_used_bytes() == 0 && total_capacity_bytes() == 0) ||
-            (!is_empty() && length() >= 0 && region_num() >= 0 &&
-              total_used_bytes() >= 0 && total_capacity_bytes() >= 0),
-            hrs_ext_msg(this, "invariant"));
-
-  guarantee((!regions_humongous() && region_num() == length()) ||
-            ( regions_humongous() && region_num() >= length()),
-            hrs_ext_msg(this, "invariant"));
+  check_mt_safety();
 
-  guarantee(!regions_empty() || total_used_bytes() == 0,
-            hrs_ext_msg(this, "invariant"));
-
-  guarantee(total_used_bytes() <= total_capacity_bytes(),
+  guarantee(( is_empty() && length() == 0 && total_capacity_bytes() == 0) ||
+            (!is_empty() && length() >= 0 && total_capacity_bytes() >= 0),
             hrs_ext_msg(this, "invariant"));
 }
 
 void HeapRegionSetBase::verify_start() {
   // See comment in verify() about MT safety and verification.
-  hrs_assert_mt_safety_ok(this);
+  check_mt_safety();
   assert(!_verify_in_progress,
          hrs_ext_msg(this, "verification should not be in progress"));
 
   // Do the basic verification first before we do the checks over the regions.
   HeapRegionSetBase::verify();
 
-  _calc_length               = 0;
-  _calc_region_num           = 0;
-  _calc_total_capacity_bytes = 0;
-  _calc_total_used_bytes     = 0;
   _verify_in_progress        = true;
 }
 
-void HeapRegionSetBase::verify_next_region(HeapRegion* hr) {
-  // See comment in verify() about MT safety and verification.
-  hrs_assert_mt_safety_ok(this);
-  assert(_verify_in_progress,
-         hrs_ext_msg(this, "verification should be in progress"));
-
-  guarantee(verify_region(hr, this), hrs_ext_msg(this, "region verification"));
-
-  _calc_length               += 1;
-  _calc_region_num           += hr->region_num();
-  _calc_total_capacity_bytes += hr->capacity();
-  _calc_total_used_bytes     += hr->used();
-}
-
 void HeapRegionSetBase::verify_end() {
   // See comment in verify() about MT safety and verification.
-  hrs_assert_mt_safety_ok(this);
+  check_mt_safety();
   assert(_verify_in_progress,
          hrs_ext_msg(this, "verification should be in progress"));
 
-  guarantee(length() == _calc_length,
-            hrs_err_msg("[%s] length: %u should be == calc length: %u",
-                        name(), length(), _calc_length));
-
-  guarantee(region_num() == _calc_region_num,
-            hrs_err_msg("[%s] region num: %u should be == calc region num: %u",
-                        name(), region_num(), _calc_region_num));
-
-  guarantee(total_capacity_bytes() == _calc_total_capacity_bytes,
-            hrs_err_msg("[%s] capacity bytes: "SIZE_FORMAT" should be == "
-                        "calc capacity bytes: "SIZE_FORMAT,
-                        name(),
-                        total_capacity_bytes(), _calc_total_capacity_bytes));
-
-  guarantee(total_used_bytes() == _calc_total_used_bytes,
-            hrs_err_msg("[%s] used bytes: "SIZE_FORMAT" should be == "
-                        "calc used bytes: "SIZE_FORMAT,
-                        name(), total_used_bytes(), _calc_total_used_bytes));
-
   _verify_in_progress = false;
 }
 
-void HeapRegionSetBase::clear_phase() {
-  assert(_phase != HRSPhaseNone, "pre-condition");
-  _phase = HRSPhaseNone;
-}
-
-void HeapRegionSetBase::set_phase(HRSPhase phase) {
-  assert(_phase == HRSPhaseNone, "pre-condition");
-  assert(phase != HRSPhaseNone, "pre-condition");
-  _phase = phase;
-}
-
 void HeapRegionSetBase::print_on(outputStream* out, bool print_contents) {
   out->cr();
   out->print_cr("Set: %s ("PTR_FORMAT")", name(), this);
@@ -196,76 +85,38 @@ void HeapRegionSetBase::print_on(outputStream* out, bool print_contents) {
   out->print_cr("    empty             : %s", BOOL_TO_STR(regions_empty()));
   out->print_cr("  Attributes");
   out->print_cr("    length            : %14u", length());
-  out->print_cr("    region num        : %14u", region_num());
   out->print_cr("    total capacity    : "SIZE_FORMAT_W(14)" bytes",
                 total_capacity_bytes());
-  out->print_cr("    total used        : "SIZE_FORMAT_W(14)" bytes",
-                total_used_bytes());
 }
 
-void HeapRegionSetBase::clear() {
-  _length           = 0;
-  _region_num       = 0;
-  _total_used_bytes = 0;
-}
-
-HeapRegionSetBase::HeapRegionSetBase(const char* name)
+HeapRegionSetBase::HeapRegionSetBase(const char* name, bool humongous, bool empty, HRSMtSafeChecker* mt_safety_checker)
   : _name(name), _verify_in_progress(false),
-    _calc_length(0), _calc_region_num(0),
-    _calc_total_capacity_bytes(0), _calc_total_used_bytes(0) { }
-
-//////////////////// HeapRegionSet ////////////////////
-
-void HeapRegionSet::update_from_proxy(HeapRegionSet* proxy_set) {
-  hrs_assert_mt_safety_ok(this);
-  hrs_assert_mt_safety_ok(proxy_set);
-  hrs_assert_sets_match(this, proxy_set);
-
-  verify_optional();
-  proxy_set->verify_optional();
-
-  if (proxy_set->is_empty()) return;
+    _is_humongous(humongous), _is_empty(empty), _mt_safety_checker(mt_safety_checker),
+    _count()
+{ }
 
-  assert(proxy_set->length() <= _length,
-         hrs_err_msg("[%s] proxy set length: %u should be <= length: %u",
-                     name(), proxy_set->length(), _length));
-  _length -= proxy_set->length();
-
-  assert(proxy_set->region_num() <= _region_num,
-         hrs_err_msg("[%s] proxy set region num: %u should be <= region num: %u",
-                     name(), proxy_set->region_num(), _region_num));
-  _region_num -= proxy_set->region_num();
-
-  assert(proxy_set->total_used_bytes() <= _total_used_bytes,
-         hrs_err_msg("[%s] proxy set used bytes: "SIZE_FORMAT" "
-                     "should be <= used bytes: "SIZE_FORMAT,
-                     name(), proxy_set->total_used_bytes(),
-                     _total_used_bytes));
-  _total_used_bytes -= proxy_set->total_used_bytes();
-
-  proxy_set->clear();
-
-  verify_optional();
-  proxy_set->verify_optional();
+void FreeRegionList::set_unrealistically_long_length(uint len) {
+  guarantee(_unrealistically_long_length == 0, "should only be set once");
+  _unrealistically_long_length = len;
 }
 
-//////////////////// HeapRegionLinkedList ////////////////////
-
-void HeapRegionLinkedList::fill_in_ext_msg_extra(hrs_ext_msg* msg) {
+void FreeRegionList::fill_in_ext_msg_extra(hrs_ext_msg* msg) {
   msg->append(" hd: "PTR_FORMAT" tl: "PTR_FORMAT, head(), tail());
 }
 
-void HeapRegionLinkedList::add_as_head(HeapRegionLinkedList* from_list) {
-  hrs_assert_mt_safety_ok(this);
-  hrs_assert_mt_safety_ok(from_list);
+void FreeRegionList::add_as_head_or_tail(FreeRegionList* from_list, bool as_head) {
+  check_mt_safety();
+  from_list->check_mt_safety();
 
   verify_optional();
   from_list->verify_optional();
 
-  if (from_list->is_empty()) return;
+  if (from_list->is_empty()) {
+    return;
+  }
 
 #ifdef ASSERT
-  HeapRegionLinkedListIterator iter(from_list);
+  FreeRegionListIterator iter(from_list);
   while (iter.more_available()) {
     HeapRegion* hr = iter.get_next();
     // In set_containing_set() we check that we either set the value
@@ -276,70 +127,43 @@ void HeapRegionLinkedList::add_as_head(HeapRegionLinkedList* from_list) {
   }
 #endif // ASSERT
 
-  if (_head != NULL) {
-    assert(length() >  0 && _tail != NULL, hrs_ext_msg(this, "invariant"));
-    from_list->_tail->set_next(_head);
-  } else {
+  if (_head == NULL) {
     assert(length() == 0 && _tail == NULL, hrs_ext_msg(this, "invariant"));
+    _head = from_list->_head;
     _tail = from_list->_tail;
+  } else {
+    assert(length() > 0 && _tail != NULL, hrs_ext_msg(this, "invariant"));
+    if (as_head) {
+      from_list->_tail->set_next(_head);
+      _head = from_list->_head;
+    } else {
+      _tail->set_next(from_list->_head);
+      _tail = from_list->_tail;
+    }
   }
-  _head = from_list->_head;
 
-  _length           += from_list->length();
-  _region_num       += from_list->region_num();
-  _total_used_bytes += from_list->total_used_bytes();
+  _count.increment(from_list->length(), from_list->total_capacity_bytes());
   from_list->clear();
 
   verify_optional();
   from_list->verify_optional();
 }
 
-void HeapRegionLinkedList::add_as_tail(HeapRegionLinkedList* from_list) {
-  hrs_assert_mt_safety_ok(this);
-  hrs_assert_mt_safety_ok(from_list);
-
-  verify_optional();
-  from_list->verify_optional();
-
-  if (from_list->is_empty()) return;
-
-#ifdef ASSERT
-  HeapRegionLinkedListIterator iter(from_list);
-  while (iter.more_available()) {
-    HeapRegion* hr = iter.get_next();
-    // In set_containing_set() we check that we either set the value
-    // from NULL to non-NULL or vice versa to catch bugs. So, we have
-    // to NULL it first before setting it to the value.
-    hr->set_containing_set(NULL);
-    hr->set_containing_set(this);
-  }
-#endif // ASSERT
-
-  if (_tail != NULL) {
-    assert(length() >  0 && _head != NULL, hrs_ext_msg(this, "invariant"));
-    _tail->set_next(from_list->_head);
-  } else {
-    assert(length() == 0 && _head == NULL, hrs_ext_msg(this, "invariant"));
-    _head = from_list->_head;
-  }
-  _tail = from_list->_tail;
-
-  _length           += from_list->length();
-  _region_num       += from_list->region_num();
-  _total_used_bytes += from_list->total_used_bytes();
-  from_list->clear();
+void FreeRegionList::add_as_head(FreeRegionList* from_list) {
+  add_as_head_or_tail(from_list, true /* as_head */);
+}
 
-  verify_optional();
-  from_list->verify_optional();
+void FreeRegionList::add_as_tail(FreeRegionList* from_list) {
+  add_as_head_or_tail(from_list, false /* as_head */);
 }
 
-void HeapRegionLinkedList::remove_all() {
-  hrs_assert_mt_safety_ok(this);
+void FreeRegionList::remove_all() {
+  check_mt_safety();
   verify_optional();
 
   HeapRegion* curr = _head;
   while (curr != NULL) {
-    hrs_assert_region_ok(this, curr, this);
+    verify_region(curr);
 
     HeapRegion* next = curr->next();
     curr->set_next(NULL);
@@ -351,8 +175,8 @@ void HeapRegionLinkedList::remove_all() {
   verify_optional();
 }
 
-void HeapRegionLinkedList::remove_all_pending(uint target_count) {
-  hrs_assert_mt_safety_ok(this);
+void FreeRegionList::remove_all_pending(uint target_count) {
+  check_mt_safety();
   assert(target_count > 1, hrs_ext_msg(this, "pre-condition"));
   assert(!is_empty(), hrs_ext_msg(this, "pre-condition"));
 
@@ -363,7 +187,7 @@ void HeapRegionLinkedList::remove_all_pending(uint target_count) {
   HeapRegion* prev = NULL;
   uint count = 0;
   while (curr != NULL) {
-    hrs_assert_region_ok(this, curr, this);
+    verify_region(curr);
     HeapRegion* next = curr->next();
 
     if (curr->pending_removal()) {
@@ -387,7 +211,7 @@ void HeapRegionLinkedList::remove_all_pending(uint target_count) {
       }
 
       curr->set_next(NULL);
-      remove_internal(curr);
+      remove(curr);
       curr->set_pending_removal(false);
 
       count += 1;
@@ -414,46 +238,26 @@ void HeapRegionLinkedList::remove_all_pending(uint target_count) {
   verify_optional();
 }
 
-void HeapRegionLinkedList::verify() {
+void FreeRegionList::verify() {
   // See comment in HeapRegionSetBase::verify() about MT safety and
   // verification.
-  hrs_assert_mt_safety_ok(this);
+  check_mt_safety();
 
   // This will also do the basic verification too.
   verify_start();
 
-  HeapRegion* curr  = _head;
-  HeapRegion* prev1 = NULL;
-  HeapRegion* prev0 = NULL;
-  uint        count = 0;
-  while (curr != NULL) {
-    verify_next_region(curr);
-
-    count += 1;
-    guarantee(count < _unrealistically_long_length,
-              hrs_err_msg("[%s] the calculated length: %u "
-                          "seems very long, is there maybe a cycle? "
-                          "curr: "PTR_FORMAT" prev0: "PTR_FORMAT" "
-                          "prev1: "PTR_FORMAT" length: %u",
-                          name(), count, curr, prev0, prev1, length()));
-
-    prev1 = prev0;
-    prev0 = curr;
-    curr  = curr->next();
-  }
-
-  guarantee(_tail == prev0, hrs_ext_msg(this, "post-condition"));
+  verify_list();
 
   verify_end();
 }
 
-void HeapRegionLinkedList::clear() {
-  HeapRegionSetBase::clear();
+void FreeRegionList::clear() {
+  _count = HeapRegionSetCount();
   _head = NULL;
   _tail = NULL;
 }
 
-void HeapRegionLinkedList::print_on(outputStream* out, bool print_contents) {
+void FreeRegionList::print_on(outputStream* out, bool print_contents) {
   HeapRegionSetBase::print_on(out, print_contents);
   out->print_cr("  Linking");
   out->print_cr("    head              : "PTR_FORMAT, _head);
@@ -461,7 +265,7 @@ void HeapRegionLinkedList::print_on(outputStream* out, bool print_contents) {
 
   if (print_contents) {
     out->print_cr("  Contents");
-    HeapRegionLinkedListIterator iter(this);
+    FreeRegionListIterator iter(this);
     while (iter.more_available()) {
       HeapRegion* hr = iter.get_next();
       hr->print_on(out);

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

@@ -38,134 +38,107 @@ typedef FormatBuffer<HRS_ERR_MSG_BUFSZ> hrs_err_msg;
 #define HEAP_REGION_SET_FORCE_VERIFY defined(ASSERT)
 #endif // HEAP_REGION_SET_FORCE_VERIFY
 
-//////////////////// HeapRegionSetBase ////////////////////
+class hrs_ext_msg;
+
+class HRSMtSafeChecker : public CHeapObj<mtGC> {
+public:
+  virtual void check() = 0;
+};
+
+class MasterFreeRegionListMtSafeChecker    : public HRSMtSafeChecker { public: void check(); };
+class SecondaryFreeRegionListMtSafeChecker : public HRSMtSafeChecker { public: void check(); };
+class HumongousRegionSetMtSafeChecker      : public HRSMtSafeChecker { public: void check(); };
+class OldRegionSetMtSafeChecker            : public HRSMtSafeChecker { public: void check(); };
+
+class HeapRegionSetCount VALUE_OBJ_CLASS_SPEC {
+  friend class VMStructs;
+  uint   _length;
+  size_t _capacity;
+
+public:
+  HeapRegionSetCount() : _length(0), _capacity(0) { }
+
+  const uint   length()   const { return _length;   }
+  const size_t capacity() const { return _capacity; }
+
+  void increment(uint length_to_add, size_t capacity_to_add) {
+    _length += length_to_add;
+    _capacity += capacity_to_add;
+  }
+
+  void decrement(const uint length_to_remove, const size_t capacity_to_remove) {
+    _length -= length_to_remove;
+    _capacity -= capacity_to_remove;
+  }
+};
 
 // Base class for all the classes that represent heap region sets. It
 // contains the basic attributes that each set needs to maintain
 // (e.g., length, region num, used bytes sum) plus any shared
 // functionality (e.g., verification).
 
-class hrs_ext_msg;
-
-typedef enum {
-  HRSPhaseNone,
-  HRSPhaseEvacuation,
-  HRSPhaseCleanup,
-  HRSPhaseFullGC
-} HRSPhase;
-
-class HRSPhaseSetter;
-
 class HeapRegionSetBase VALUE_OBJ_CLASS_SPEC {
-  friend class hrs_ext_msg;
-  friend class HRSPhaseSetter;
   friend class VMStructs;
+private:
+  bool _is_humongous;
+  bool _is_empty;
+  HRSMtSafeChecker* _mt_safety_checker;
 
 protected:
-  static uint _unrealistically_long_length;
-
   // The number of regions added to the set. If the set contains
   // only humongous regions, this reflects only 'starts humongous'
   // regions and does not include 'continues humongous' ones.
-  uint _length;
-
-  // The total number of regions represented by the set. If the set
-  // does not contain humongous regions, this should be the same as
-  // _length. If the set contains only humongous regions, this will
-  // include the 'continues humongous' regions.
-  uint _region_num;
-
-  // We don't keep track of the total capacity explicitly, we instead
-  // recalculate it based on _region_num and the heap region size.
-
-  // The sum of used bytes in the all the regions in the set.
-  size_t _total_used_bytes;
+  HeapRegionSetCount _count;
 
   const char* _name;
 
-  bool        _verify_in_progress;
-  uint        _calc_length;
-  uint        _calc_region_num;
-  size_t      _calc_total_capacity_bytes;
-  size_t      _calc_total_used_bytes;
-
-  // This is here so that it can be used in the subclasses to assert
-  // something different depending on which phase the GC is in. This
-  // can be particularly helpful in the check_mt_safety() methods.
-  static HRSPhase _phase;
-
-  // Only used by HRSPhaseSetter.
-  static void clear_phase();
-  static void set_phase(HRSPhase phase);
+  bool _verify_in_progress;
 
   // verify_region() is used to ensure that the contents of a region
-  // added to / removed from a set are consistent. Different sets
-  // make different assumptions about the regions added to them. So
-  // each set can override verify_region_extra(), which is called
-  // from verify_region(), and do any extra verification it needs to
-  // perform in that.
-  virtual const char* verify_region_extra(HeapRegion* hr) { return NULL; }
-  bool verify_region(HeapRegion* hr,
-                     HeapRegionSetBase* expected_containing_set);
+  // added to / removed from a set are consistent.
+  void verify_region(HeapRegion* hr) PRODUCT_RETURN;
 
   // Indicates whether all regions in the set should be humongous or
   // not. Only used during verification.
-  virtual bool regions_humongous() = 0;
+  bool regions_humongous() { return _is_humongous; }
 
   // Indicates whether all regions in the set should be empty or
   // not. Only used during verification.
-  virtual bool regions_empty() = 0;
+  bool regions_empty() { return _is_empty; }
 
-  // Subclasses can optionally override this to do MT safety protocol
-  // checks. It is called in an assert from all methods that perform
-  // updates on the set (and subclasses should also call it too).
-  virtual bool check_mt_safety() { return true; }
+  void check_mt_safety() {
+    if (_mt_safety_checker != NULL) {
+      _mt_safety_checker->check();
+    }
+  }
 
-  // fill_in_ext_msg() writes the the values of the set's attributes
-  // in the custom err_msg (hrs_ext_msg). fill_in_ext_msg_extra()
-  // allows subclasses to append further information.
   virtual void fill_in_ext_msg_extra(hrs_ext_msg* msg) { }
-  void fill_in_ext_msg(hrs_ext_msg* msg, const char* message);
-
-  // It updates the fields of the set to reflect hr being added to
-  // the set.
-  inline void update_for_addition(HeapRegion* hr);
 
-  // It updates the fields of the set to reflect hr being added to
-  // the set and tags the region appropriately.
-  inline void add_internal(HeapRegion* hr);
-
-  // It updates the fields of the set to reflect hr being removed
-  // from the set.
-  inline void update_for_removal(HeapRegion* hr);
-
-  // It updates the fields of the set to reflect hr being removed
-  // from the set and tags the region appropriately.
-  inline void remove_internal(HeapRegion* hr);
-
-  // It clears all the fields of the sets. Note: it will not iterate
-  // over the set and remove regions from it. It assumes that the
-  // caller has already done so. It will literally just clear the fields.
-  virtual void clear();
-
-  HeapRegionSetBase(const char* name);
+  HeapRegionSetBase(const char* name, bool humongous, bool empty, HRSMtSafeChecker* mt_safety_checker);
 
 public:
-  static void set_unrealistically_long_length(uint len);
-
   const char* name() { return _name; }
 
-  uint length() { return _length; }
+  uint length() { return _count.length(); }
 
-  bool is_empty() { return _length == 0; }
-
-  uint region_num() { return _region_num; }
+  bool is_empty() { return _count.length() == 0; }
 
   size_t total_capacity_bytes() {
-    return (size_t) region_num() << HeapRegion::LogOfHRGrainBytes;
+    return _count.capacity();
   }
 
-  size_t total_used_bytes() { return _total_used_bytes; }
+  // It updates the fields of the set to reflect hr being added to
+  // the set and tags the region appropriately.
+  inline void add(HeapRegion* hr);
+
+  // It updates the fields of the set to reflect hr being removed
+  // from the set and tags the region appropriately.
+  inline void remove(HeapRegion* hr);
+
+  // fill_in_ext_msg() writes the the values of the set's attributes
+  // in the custom err_msg (hrs_ext_msg). fill_in_ext_msg_extra()
+  // allows subclasses to append further information.
+  void fill_in_ext_msg(hrs_ext_msg* msg, const char* message);
 
   virtual void verify();
   void verify_start();
@@ -187,7 +160,6 @@ public:
 // assert/guarantee-specific message it also prints out the values of
 // the fields of the associated set. This can be very helpful in
 // diagnosing failures.
-
 class hrs_ext_msg : public hrs_err_msg {
 public:
   hrs_ext_msg(HeapRegionSetBase* set, const char* message) : hrs_err_msg("") {
@@ -195,32 +167,6 @@ public:
   }
 };
 
-class HRSPhaseSetter {
-public:
-  HRSPhaseSetter(HRSPhase phase) {
-    HeapRegionSetBase::set_phase(phase);
-  }
-  ~HRSPhaseSetter() {
-    HeapRegionSetBase::clear_phase();
-  }
-};
-
-// These two macros are provided for convenience, to keep the uses of
-// these two asserts a bit more concise.
-
-#define hrs_assert_mt_safety_ok(_set_)                                        \
-  do {                                                                        \
-    assert((_set_)->check_mt_safety(), hrs_ext_msg((_set_), "MT safety"));    \
-  } while (0)
-
-#define hrs_assert_region_ok(_set_, _hr_, _expected_)                         \
-  do {                                                                        \
-    assert((_set_)->verify_region((_hr_), (_expected_)),                      \
-           hrs_ext_msg((_set_), "region verification"));                      \
-  } while (0)
-
-//////////////////// HeapRegionSet ////////////////////
-
 #define hrs_assert_sets_match(_set1_, _set2_)                                 \
   do {                                                                        \
     assert(((_set1_)->regions_humongous() ==                                  \
@@ -236,63 +182,33 @@ public:
 // the same interface (namely, the HeapRegionSetBase API).
 
 class HeapRegionSet : public HeapRegionSetBase {
-protected:
-  virtual const char* verify_region_extra(HeapRegion* hr) {
-    if (hr->next() != NULL) {
-      return "next() should always be NULL as we do not link the regions";
-    }
-
-    return HeapRegionSetBase::verify_region_extra(hr);
-  }
-
-  HeapRegionSet(const char* name) : HeapRegionSetBase(name) {
-    clear();
-  }
-
 public:
-  // It adds hr to the set. The region should not be a member of
-  // another set.
-  inline void add(HeapRegion* hr);
-
-  // It removes hr from the set. The region should be a member of
-  // this set.
-  inline void remove(HeapRegion* hr);
+  HeapRegionSet(const char* name, bool humongous, HRSMtSafeChecker* mt_safety_checker):
+    HeapRegionSetBase(name, humongous, false /* empty */, mt_safety_checker) { }
 
-  // It removes a region from the set. Instead of updating the fields
-  // of the set to reflect this removal, it accumulates the updates
-  // in proxy_set. The idea is that proxy_set is thread-local to
-  // avoid multiple threads updating the fields of the set
-  // concurrently and having to synchronize. The method
-  // update_from_proxy() will update the fields of the set from the
-  // proxy_set.
-  inline void remove_with_proxy(HeapRegion* hr, HeapRegionSet* proxy_set);
-
-  // After multiple calls to remove_with_proxy() the updates to the
-  // fields of the set are accumulated in proxy_set. This call
-  // updates the fields of the set from proxy_set.
-  void update_from_proxy(HeapRegionSet* proxy_set);
+  void bulk_remove(const HeapRegionSetCount& removed) {
+    _count.decrement(removed.length(), removed.capacity());
+  }
 };
 
-//////////////////// HeapRegionLinkedList ////////////////////
-
 // A set that links all the regions added to it in a singly-linked
 // list. We should try to avoid doing operations that iterate over
 // such lists in performance critical paths. Typically we should
 // add / remove one region at a time or concatenate two lists. All
 // those operations are done in constant time.
 
-class HeapRegionLinkedListIterator;
+class FreeRegionListIterator;
 
-class HeapRegionLinkedList : public HeapRegionSetBase {
-  friend class HeapRegionLinkedListIterator;
+class FreeRegionList : public HeapRegionSetBase {
+  friend class FreeRegionListIterator;
 
 private:
   HeapRegion* _head;
   HeapRegion* _tail;
 
-  // These are provided for use by the friend classes.
-  HeapRegion* head() { return _head; }
-  HeapRegion* tail() { return _tail; }
+  static uint _unrealistically_long_length;
+
+  void add_as_head_or_tail(FreeRegionList* from_list, bool as_head);
 
 protected:
   virtual void fill_in_ext_msg_extra(hrs_ext_msg* msg);
@@ -300,11 +216,19 @@ protected:
   // See the comment for HeapRegionSetBase::clear()
   virtual void clear();
 
-  HeapRegionLinkedList(const char* name) : HeapRegionSetBase(name) {
+public:
+  FreeRegionList(const char* name, HRSMtSafeChecker* mt_safety_checker = NULL):
+    HeapRegionSetBase(name, false /* humongous */, true /* empty */, mt_safety_checker) {
     clear();
   }
 
-public:
+  void verify_list();
+
+  HeapRegion* head() { return _head; }
+  HeapRegion* tail() { return _tail; }
+
+  static void set_unrealistically_long_length(uint len);
+
   // It adds hr to the list as the new head. The region should not be
   // a member of another set.
   inline void add_as_head(HeapRegion* hr);
@@ -323,12 +247,12 @@ public:
   // It moves the regions from from_list to this list and empties
   // from_list. The new regions will appear in the same order as they
   // were in from_list and be linked in the beginning of this list.
-  void add_as_head(HeapRegionLinkedList* from_list);
+  void add_as_head(FreeRegionList* from_list);
 
   // It moves the regions from from_list to this list and empties
   // from_list. The new regions will appear in the same order as they
   // were in from_list and be linked in the end of this list.
-  void add_as_tail(HeapRegionLinkedList* from_list);
+  void add_as_tail(FreeRegionList* from_list);
 
   // It empties the list by removing all regions from it.
   void remove_all();
@@ -346,14 +270,12 @@ public:
   virtual void print_on(outputStream* out, bool print_contents = false);
 };
 
-//////////////////// HeapRegionLinkedListIterator ////////////////////
-
 // Iterator class that provides a convenient way to iterate over the
 // regions of a HeapRegionLinkedList instance.
 
-class HeapRegionLinkedListIterator : public StackObj {
+class FreeRegionListIterator : public StackObj {
 private:
-  HeapRegionLinkedList* _list;
+  FreeRegionList* _list;
   HeapRegion*           _curr;
 
 public:
@@ -369,12 +291,12 @@ public:
     // do the "cycle" check.
 
     HeapRegion* hr = _curr;
-    assert(_list->verify_region(hr, _list), "region verification");
+    _list->verify_region(hr);
     _curr = hr->next();
     return hr;
   }
 
-  HeapRegionLinkedListIterator(HeapRegionLinkedList* list)
+  FreeRegionListIterator(FreeRegionList* list)
     : _curr(NULL), _list(list) {
     _curr = list->head();
   }

src/share/vm/gc_implementation/g1/heapRegionSet.inline.hpp

@@ -27,87 +27,32 @@
 
 #include "gc_implementation/g1/heapRegionSet.hpp"
 
-//////////////////// HeapRegionSetBase ////////////////////
-
-inline void HeapRegionSetBase::update_for_addition(HeapRegion* hr) {
-  // Assumes the caller has already verified the region.
-
-  _length           += 1;
-  _region_num       += hr->region_num();
-  _total_used_bytes += hr->used();
-}
-
-inline void HeapRegionSetBase::add_internal(HeapRegion* hr) {
-  hrs_assert_region_ok(this, hr, NULL);
+inline void HeapRegionSetBase::add(HeapRegion* hr) {
+  check_mt_safety();
+  assert(hr->containing_set() == NULL, hrs_ext_msg(this, "should not already have a containing set %u"));
   assert(hr->next() == NULL, hrs_ext_msg(this, "should not already be linked"));
 
-  update_for_addition(hr);
+  _count.increment(1u, hr->capacity());
   hr->set_containing_set(this);
+  verify_region(hr);
 }
 
-inline void HeapRegionSetBase::update_for_removal(HeapRegion* hr) {
-  // Assumes the caller has already verified the region.
-  assert(_length > 0, hrs_ext_msg(this, "pre-condition"));
-  _length -= 1;
-
-  uint region_num_diff = hr->region_num();
-  assert(region_num_diff <= _region_num,
-         hrs_err_msg("[%s] region's region num: %u "
-                     "should be <= region num: %u",
-                     name(), region_num_diff, _region_num));
-  _region_num -= region_num_diff;
-
-  size_t used_bytes = hr->used();
-  assert(used_bytes <= _total_used_bytes,
-         hrs_err_msg("[%s] region's used bytes: "SIZE_FORMAT" "
-                     "should be <= used bytes: "SIZE_FORMAT,
-                     name(), used_bytes, _total_used_bytes));
-  _total_used_bytes -= used_bytes;
-}
-
-inline void HeapRegionSetBase::remove_internal(HeapRegion* hr) {
-  hrs_assert_region_ok(this, hr, this);
+inline void HeapRegionSetBase::remove(HeapRegion* hr) {
+  check_mt_safety();
+  verify_region(hr);
   assert(hr->next() == NULL, hrs_ext_msg(this, "should already be unlinked"));
 
   hr->set_containing_set(NULL);
-  update_for_removal(hr);
+  assert(_count.length() > 0, hrs_ext_msg(this, "pre-condition"));
+  _count.decrement(1u, hr->capacity());
 }
 
-//////////////////// HeapRegionSet ////////////////////
-
-inline void HeapRegionSet::add(HeapRegion* hr) {
-  hrs_assert_mt_safety_ok(this);
-  // add_internal() will verify the region.
-  add_internal(hr);
-}
-
-inline void HeapRegionSet::remove(HeapRegion* hr) {
-  hrs_assert_mt_safety_ok(this);
-  // remove_internal() will verify the region.
-  remove_internal(hr);
-}
-
-inline void HeapRegionSet::remove_with_proxy(HeapRegion* hr,
-                                             HeapRegionSet* proxy_set) {
-  // No need to fo the MT safety check here given that this method
-  // does not update the contents of the set but instead accumulates
-  // the changes in proxy_set which is assumed to be thread-local.
-  hrs_assert_sets_match(this, proxy_set);
-  hrs_assert_region_ok(this, hr, this);
-
-  hr->set_containing_set(NULL);
-  proxy_set->update_for_addition(hr);
-}
-
-//////////////////// HeapRegionLinkedList ////////////////////
-
-inline void HeapRegionLinkedList::add_as_head(HeapRegion* hr) {
-  hrs_assert_mt_safety_ok(this);
+inline void FreeRegionList::add_as_head(HeapRegion* hr) {
   assert((length() == 0 && _head == NULL && _tail == NULL) ||
          (length() >  0 && _head != NULL && _tail != NULL),
          hrs_ext_msg(this, "invariant"));
-  // add_internal() will verify the region.
-  add_internal(hr);
+  // add() will verify the region and check mt safety.
+  add(hr);
 
   // Now link the region.
   if (_head != NULL) {
@@ -118,13 +63,13 @@ inline void HeapRegionLinkedList::add_as_head(HeapRegion* hr) {
   _head = hr;
 }
 
-inline void HeapRegionLinkedList::add_as_tail(HeapRegion* hr) {
-  hrs_assert_mt_safety_ok(this);
+inline void FreeRegionList::add_as_tail(HeapRegion* hr) {
+  check_mt_safety();
   assert((length() == 0 && _head == NULL && _tail == NULL) ||
          (length() >  0 && _head != NULL && _tail != NULL),
          hrs_ext_msg(this, "invariant"));
-  // add_internal() will verify the region.
-  add_internal(hr);
+  // add() will verify the region and check mt safety
+  add(hr);
 
   // Now link the region.
   if (_tail != NULL) {
@@ -135,8 +80,7 @@ inline void HeapRegionLinkedList::add_as_tail(HeapRegion* hr) {
   _tail = hr;
 }
 
-inline HeapRegion* HeapRegionLinkedList::remove_head() {
-  hrs_assert_mt_safety_ok(this);
+inline HeapRegion* FreeRegionList::remove_head() {
   assert(!is_empty(), hrs_ext_msg(this, "the list should not be empty"));
   assert(length() > 0 && _head != NULL && _tail != NULL,
          hrs_ext_msg(this, "invariant"));
@@ -149,14 +93,13 @@ inline HeapRegion* HeapRegionLinkedList::remove_head() {
   }
   hr->set_next(NULL);
 
-  // remove_internal() will verify the region.
-  remove_internal(hr);
+  // remove() will verify the region and check mt safety
+  remove(hr);
   return hr;
 }
 
-inline HeapRegion* HeapRegionLinkedList::remove_head_or_null() {
-  hrs_assert_mt_safety_ok(this);
-
+inline HeapRegion* FreeRegionList::remove_head_or_null() {
+  check_mt_safety();
   if (!is_empty()) {
     return remove_head();
   } else {

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

@@ -24,7 +24,7 @@
 
 #include "precompiled.hpp"
 #include "gc_implementation/g1/heapRegionRemSet.hpp"
-#include "gc_implementation/g1/heapRegionSets.hpp"
+#include "gc_implementation/g1/heapRegionSet.hpp"
 
 // Note on the check_mt_safety() methods below:
 //
@@ -37,30 +37,34 @@
 // for the "master" heap region sets / lists, the check_mt_safety()
 // method should include the VM thread / STW case.
 
-//////////////////// FreeRegionList ////////////////////
+void FreeRegionList::verify_list() {
+  HeapRegion* curr = head();
+  HeapRegion* prev1 = NULL;
+  HeapRegion* prev0 = NULL;
+  uint count = 0;
+  size_t capacity = 0;
+  while (curr != NULL) {
+    verify_region(curr);
 
-const char* FreeRegionList::verify_region_extra(HeapRegion* hr) {
-  if (hr->is_young()) {
-    return "the region should not be young";
-  }
-  // The superclass will check that the region is empty and
-  // not humongous.
-  return HeapRegionLinkedList::verify_region_extra(hr);
-}
+    count++;
+    guarantee(count < _unrealistically_long_length,
+        hrs_err_msg("[%s] the calculated length: %u seems very long, is there maybe a cycle? curr: "PTR_FORMAT" prev0: "PTR_FORMAT" " "prev1: "PTR_FORMAT" length: %u", name(), count, curr, prev0, prev1, length()));
 
-//////////////////// MasterFreeRegionList ////////////////////
+    capacity += curr->capacity();
 
-const char* MasterFreeRegionList::verify_region_extra(HeapRegion* hr) {
-  // We should reset the RSet for parallel iteration before we add it
-  // to the master free list so that it is ready when the region is
-  // re-allocated.
-  if (!hr->rem_set()->verify_ready_for_par_iteration()) {
-    return "the region's RSet should be ready for parallel iteration";
+    prev1 = prev0;
+    prev0 = curr;
+    curr = curr->next();
   }
-  return FreeRegionList::verify_region_extra(hr);
+
+  guarantee(tail() == prev0, err_msg("Expected %s to end with %u but it ended with %u.", name(), tail()->hrs_index(), prev0->hrs_index()));
+
+  guarantee(length() == count, err_msg("%s count mismatch. Expected %u, actual %u.", name(), length(), count));
+  guarantee(total_capacity_bytes() == capacity, err_msg("%s capacity mismatch. Expected " SIZE_FORMAT ", actual " SIZE_FORMAT,
+      name(), total_capacity_bytes(), capacity));
 }
 
-bool MasterFreeRegionList::check_mt_safety() {
+void MasterFreeRegionListMtSafeChecker::check() {
   // Master Free List MT safety protocol:
   // (a) If we're at a safepoint, operations on the master free list
   // should be invoked by either the VM thread (which will serialize
@@ -71,45 +75,21 @@ bool MasterFreeRegionList::check_mt_safety() {
 
   if (SafepointSynchronize::is_at_safepoint()) {
     guarantee(Thread::current()->is_VM_thread() ||
-              FreeList_lock->owned_by_self(),
-              hrs_ext_msg(this, "master free list MT safety protocol "
-                                "at a safepoint"));
+              FreeList_lock->owned_by_self(), "master free list MT safety protocol at a safepoint");
   } else {
-    guarantee(Heap_lock->owned_by_self(),
-              hrs_ext_msg(this, "master free list MT safety protocol "
-                                "outside a safepoint"));
+    guarantee(Heap_lock->owned_by_self(), "master free list MT safety protocol outside a safepoint");
   }
-
-  return FreeRegionList::check_mt_safety();
 }
 
-//////////////////// SecondaryFreeRegionList ////////////////////
-
-bool SecondaryFreeRegionList::check_mt_safety() {
+void SecondaryFreeRegionListMtSafeChecker::check() {
   // Secondary Free List MT safety protocol:
   // Operations on the secondary free list should always be invoked
   // while holding the SecondaryFreeList_lock.
 
-  guarantee(SecondaryFreeList_lock->owned_by_self(),
-            hrs_ext_msg(this, "secondary free list MT safety protocol"));
-
-  return FreeRegionList::check_mt_safety();
-}
-
-//////////////////// OldRegionSet ////////////////////
-
-const char* OldRegionSet::verify_region_extra(HeapRegion* hr) {
-  if (hr->is_young()) {
-    return "the region should not be young";
-  }
-  // The superclass will check that the region is not empty and not
-  // humongous.
-  return HeapRegionSet::verify_region_extra(hr);
+  guarantee(SecondaryFreeList_lock->owned_by_self(), "secondary free list MT safety protocol");
 }
 
-//////////////////// MasterOldRegionSet ////////////////////
-
-bool MasterOldRegionSet::check_mt_safety() {
+void OldRegionSetMtSafeChecker::check() {
   // Master Old Set MT safety protocol:
   // (a) If we're at a safepoint, operations on the master old set
   // should be invoked:
@@ -124,35 +104,16 @@ bool MasterOldRegionSet::check_mt_safety() {
   // should be invoked while holding the Heap_lock.
 
   if (SafepointSynchronize::is_at_safepoint()) {
-    guarantee(Thread::current()->is_VM_thread() ||
-              _phase == HRSPhaseEvacuation && FreeList_lock->owned_by_self() ||
-              _phase == HRSPhaseCleanup && OldSets_lock->owned_by_self(),
-              hrs_ext_msg(this, "master old set MT safety protocol "
-                                "at a safepoint"));
+    guarantee(Thread::current()->is_VM_thread()
+        || FreeList_lock->owned_by_self() || OldSets_lock->owned_by_self(),
+        "master old set MT safety protocol at a safepoint");
   } else {
-    guarantee(Heap_lock->owned_by_self(),
-              hrs_ext_msg(this, "master old set MT safety protocol "
-                                "outside a safepoint"));
+    guarantee(Heap_lock->owned_by_self(), "master old set MT safety protocol outside a safepoint");
   }
-
-  return OldRegionSet::check_mt_safety();
 }
 
-//////////////////// HumongousRegionSet ////////////////////
-
-const char* HumongousRegionSet::verify_region_extra(HeapRegion* hr) {
-  if (hr->is_young()) {
-    return "the region should not be young";
-  }
-  // The superclass will check that the region is not empty and
-  // humongous.
-  return HeapRegionSet::verify_region_extra(hr);
-}
-
-//////////////////// MasterHumongousRegionSet ////////////////////
-
-bool MasterHumongousRegionSet::check_mt_safety() {
-  // Master Humongous Set MT safety protocol:
+void HumongousRegionSetMtSafeChecker::check() {
+  // Humongous Set MT safety protocol:
   // (a) If we're at a safepoint, operations on the master humongous
   // set should be invoked by either the VM thread (which will
   // serialize them) or by the GC workers while holding the
@@ -163,13 +124,9 @@ bool MasterHumongousRegionSet::check_mt_safety() {
   if (SafepointSynchronize::is_at_safepoint()) {
     guarantee(Thread::current()->is_VM_thread() ||
               OldSets_lock->owned_by_self(),
-              hrs_ext_msg(this, "master humongous set MT safety protocol "
-                                "at a safepoint"));
+              "master humongous set MT safety protocol at a safepoint");
   } else {
     guarantee(Heap_lock->owned_by_self(),
-              hrs_ext_msg(this, "master humongous set MT safety protocol "
-                                "outside a safepoint"));
+              "master humongous set MT safety protocol outside a safepoint");
   }
-
-  return HumongousRegionSet::check_mt_safety();
 }

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

-/*
- * Copyright (c) 2011, Oracle and/or its affiliates. All rights reserved.
- * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
- *
- * This code is free software; you can redistribute it and/or modify it
- * under the terms of the GNU General Public License version 2 only, as
- * published by the Free Software Foundation.
- *
- * This code is distributed in the hope that it will be useful, but WITHOUT
- * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
- * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
- * version 2 for more details (a copy is included in the LICENSE file that
- * accompanied this code).
- *
- * You should have received a copy of the GNU General Public License version
- * 2 along with this work; if not, write to the Free Software Foundation,
- * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
- *
- * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
- * or visit www.oracle.com if you need additional information or have any
- * questions.
- *
- */
-
-#ifndef SHARE_VM_GC_IMPLEMENTATION_G1_HEAPREGIONSETS_HPP
-#define SHARE_VM_GC_IMPLEMENTATION_G1_HEAPREGIONSETS_HPP
-
-#include "gc_implementation/g1/heapRegionSet.inline.hpp"
-
-//////////////////// FreeRegionList ////////////////////
-
-class FreeRegionList : public HeapRegionLinkedList {
-protected:
-  virtual const char* verify_region_extra(HeapRegion* hr);
-
-  virtual bool regions_humongous() { return false; }
-  virtual bool regions_empty()     { return true;  }
-
-public:
-  FreeRegionList(const char* name) : HeapRegionLinkedList(name) { }
-};
-
-//////////////////// MasterFreeRegionList ////////////////////
-
-class MasterFreeRegionList : public FreeRegionList {
-protected:
-  virtual const char* verify_region_extra(HeapRegion* hr);
-  virtual bool check_mt_safety();
-
-public:
-  MasterFreeRegionList(const char* name) : FreeRegionList(name) { }
-};
-
-//////////////////// SecondaryFreeRegionList ////////////////////
-
-class SecondaryFreeRegionList : public FreeRegionList {
-protected:
-  virtual bool check_mt_safety();
-
-public:
-  SecondaryFreeRegionList(const char* name) : FreeRegionList(name) { }
-};
-
-//////////////////// OldRegionSet ////////////////////
-
-class OldRegionSet : public HeapRegionSet {
-protected:
-  virtual const char* verify_region_extra(HeapRegion* hr);
-
-  virtual bool regions_humongous() { return false; }
-  virtual bool regions_empty()     { return false; }
-
-public:
-  OldRegionSet(const char* name) : HeapRegionSet(name) { }
-};
-
-//////////////////// MasterOldRegionSet ////////////////////
-
-class MasterOldRegionSet : public OldRegionSet {
-private:
-protected:
-  virtual bool check_mt_safety();
-
-public:
-  MasterOldRegionSet(const char* name) : OldRegionSet(name) { }
-};
-
-//////////////////// HumongousRegionSet ////////////////////
-
-class HumongousRegionSet : public HeapRegionSet {
-protected:
-  virtual const char* verify_region_extra(HeapRegion* hr);
-
-  virtual bool regions_humongous() { return true;  }
-  virtual bool regions_empty()     { return false; }
-
-public:
-  HumongousRegionSet(const char* name) : HeapRegionSet(name) { }
-};
-
-//////////////////// MasterHumongousRegionSet ////////////////////
-
-class MasterHumongousRegionSet : public HumongousRegionSet {
-protected:
-  virtual bool check_mt_safety();
-
-public:
-  MasterHumongousRegionSet(const char* name) : HumongousRegionSet(name) { }
-};
-
-#endif // SHARE_VM_GC_IMPLEMENTATION_G1_HEAPREGIONSETS_HPP

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

@@ -57,9 +57,10 @@
   nonstatic_field(G1MonitoringSupport, _old_committed,      size_t)           \
   nonstatic_field(G1MonitoringSupport, _old_used,           size_t)           \
                                                                               \
-  nonstatic_field(HeapRegionSetBase,   _length,             uint)             \
-  nonstatic_field(HeapRegionSetBase,   _region_num,         uint)             \
-  nonstatic_field(HeapRegionSetBase,   _total_used_bytes,   size_t)           \
+  nonstatic_field(HeapRegionSetBase,   _count,          HeapRegionSetCount)   \
+                                                                              \
+  nonstatic_field(HeapRegionSetCount,  _length,         uint)                 \
+  nonstatic_field(HeapRegionSetCount,  _capacity,       size_t)               \
 
 
 #define VM_TYPES_G1(declare_type, declare_toplevel_type)                      \
@@ -71,6 +72,7 @@
   declare_type(HeapRegion, ContiguousSpace)                                   \
   declare_toplevel_type(HeapRegionSeq)                                        \
   declare_toplevel_type(HeapRegionSetBase)                                    \
+  declare_toplevel_type(HeapRegionSetCount)                                   \
   declare_toplevel_type(G1MonitoringSupport)                                  \
                                                                               \
   declare_toplevel_type(G1CollectedHeap*)                                     \