8150393: Maintain the set of survivor regions in an array between GCs

Reviewed-by: tschatzl, sjohanss

hotspot/src/share/vm/gc/g1/g1CollectionSet.cpp

@@ -301,15 +301,17 @@ double G1CollectionSet::finalize_young_part(double target_pause_time_ms) {
   uint eden_region_length = young_list->eden_length();
   init_region_lengths(eden_region_length, survivor_region_length);
 
-  HeapRegion* hr = young_list->first_survivor_region();
+  const GrowableArray<HeapRegion*>* survivor_regions = _g1->young_list()->survivor_regions();
-  while (hr != NULL) {
+  for (GrowableArrayIterator<HeapRegion*> it = survivor_regions->begin();
+       it != survivor_regions->end();
+       ++it) {
+    HeapRegion* hr = *it;
     assert(hr->is_survivor(), "badly formed young list");
     // There is a convention that all the young regions in the CSet
     // are tagged as "eden", so we do this for the survivors here. We
     // use the special set_eden_pre_gc() as it doesn't check that the
     // region is free (which is not the case here).
     hr->set_eden_pre_gc();
-    hr = hr->get_next_young_region();
   }
 
   verify_young_cset_indices();

hotspot/src/share/vm/gc/g1/g1ConcurrentMark.cpp

@@ -57,6 +57,7 @@
 #include "runtime/java.hpp"
 #include "runtime/prefetch.inline.hpp"
 #include "services/memTracker.hpp"
+#include "utilities/growableArray.hpp"
 
 // Concurrent marking bit map wrapper
 
@@ -261,7 +262,7 @@ void G1CMMarkStack::note_end_of_gc() {
 
 G1CMRootRegions::G1CMRootRegions() :
   _young_list(NULL), _cm(NULL), _scan_in_progress(false),
-  _should_abort(false),  _next_survivor(NULL) { }
+  _should_abort(false), _claimed_survivor_index(0) { }
 
 void G1CMRootRegions::init(G1CollectedHeap* g1h, G1ConcurrentMark* cm) {
   _young_list = g1h->young_list();
@@ -272,9 +273,8 @@ void G1CMRootRegions::prepare_for_scan() {
   assert(!scan_in_progress(), "pre-condition");
 
   // Currently, only survivors can be root regions.
-  assert(_next_survivor == NULL, "pre-condition");
+  _claimed_survivor_index = 0;
-  _next_survivor = _young_list->first_survivor_region();
+  _scan_in_progress = true;
-  _scan_in_progress = (_next_survivor != NULL);
   _should_abort = false;
 }
 
@@ -286,27 +286,13 @@ HeapRegion* G1CMRootRegions::claim_next() {
   }
 
   // Currently, only survivors can be root regions.
-  HeapRegion* res = _next_survivor;
+  const GrowableArray<HeapRegion*>* survivor_regions = _young_list->survivor_regions();
-  if (res != NULL) {
-    MutexLockerEx x(RootRegionScan_lock, Mutex::_no_safepoint_check_flag);
-    // Read it again in case it changed while we were waiting for the lock.
-    res = _next_survivor;
-    if (res != NULL) {
-      if (res == _young_list->last_survivor_region()) {
-        // We just claimed the last survivor so store NULL to indicate
-        // that we're done.
-        _next_survivor = NULL;
-      } else {
-        _next_survivor = res->get_next_young_region();
-      }
-    } else {
-      // Someone else claimed the last survivor while we were trying
-      // to take the lock so nothing else to do.
-    }
-  }
-  assert(res == NULL || res->is_survivor(), "post-condition");
 
-  return res;
+  int claimed_index = Atomic::add(1, &_claimed_survivor_index) - 1;
+  if (claimed_index < survivor_regions->length()) {
+    return survivor_regions->at(claimed_index);
+  }
+  return NULL;
 }
 
 void G1CMRootRegions::notify_scan_done() {
@@ -324,9 +310,10 @@ void G1CMRootRegions::scan_finished() {
 
   // Currently, only survivors can be root regions.
   if (!_should_abort) {
-    assert(_next_survivor == NULL, "we should have claimed all survivors");
+    assert(_claimed_survivor_index >= _young_list->survivor_regions()->length(),
+           "we should have claimed all survivors, claimed index = %d, length = %d",
+           _claimed_survivor_index, _young_list->survivor_regions()->length());
   }
-  _next_survivor = NULL;
 
   notify_scan_done();
 }

hotspot/src/share/vm/gc/g1/g1ConcurrentMark.hpp

@@ -226,7 +226,7 @@ private:
 
   volatile bool        _scan_in_progress;
   volatile bool        _should_abort;
-  HeapRegion* volatile _next_survivor;
+  volatile int         _claimed_survivor_index;
 
   void notify_scan_done();
 

hotspot/src/share/vm/gc/g1/g1DefaultPolicy.cpp

@@ -41,6 +41,7 @@
 #include "runtime/java.hpp"
 #include "runtime/mutexLocker.hpp"
 #include "utilities/debug.hpp"
+#include "utilities/growableArray.hpp"
 #include "utilities/pair.hpp"
 
 G1DefaultPolicy::G1DefaultPolicy() :
@@ -358,10 +359,12 @@ G1DefaultPolicy::calculate_young_list_target_length(size_t rs_lengths,
 
 double G1DefaultPolicy::predict_survivor_regions_evac_time() const {
   double survivor_regions_evac_time = 0.0;
-  for (HeapRegion * r = _g1->young_list()->first_survivor_region();
+  const GrowableArray<HeapRegion*>* survivor_regions = _g1->young_list()->survivor_regions();
-       r != NULL && r != _g1->young_list()->last_survivor_region()->get_next_young_region();
+
-       r = r->get_next_young_region()) {
+  for (GrowableArrayIterator<HeapRegion*> it = survivor_regions->begin();
-    survivor_regions_evac_time += predict_region_elapsed_time_ms(r, collector_state()->gcs_are_young());
+       it != survivor_regions->end();
+       ++it) {
+    survivor_regions_evac_time += predict_region_elapsed_time_ms(*it, collector_state()->gcs_are_young());
   }
   return survivor_regions_evac_time;
 }

hotspot/src/share/vm/gc/g1/youngList.cpp

@@ -31,11 +31,14 @@
 #include "gc/g1/heapRegionRemSet.hpp"
 #include "gc/g1/youngList.hpp"
 #include "logging/log.hpp"
+#include "utilities/growableArray.hpp"
 #include "utilities/ostream.hpp"
 
 YoungList::YoungList(G1CollectedHeap* g1h) :
-    _g1h(g1h), _head(NULL), _length(0),
+    _g1h(g1h),
-    _survivor_head(NULL), _survivor_tail(NULL), _survivor_length(0) {
+    _survivor_regions(new (ResourceObj::C_HEAP, mtGC) GrowableArray<HeapRegion*>(8, true, mtGC)),
+    _head(NULL),
+    _length(0) {
   guarantee(check_list_empty(), "just making sure...");
 }
 
@@ -54,12 +57,7 @@ void YoungList::add_survivor_region(HeapRegion* hr) {
   assert(hr->is_survivor(), "should be flagged as survivor region");
   assert(hr->get_next_young_region() == NULL, "cause it should!");
 
-  hr->set_next_young_region(_survivor_head);
+  _survivor_regions->append(hr);
-  if (_survivor_head == NULL) {
-    _survivor_tail = hr;
-  }
-  _survivor_head = hr;
-  ++_survivor_length;
 }
 
 void YoungList::empty_list(HeapRegion* list) {
@@ -82,14 +80,18 @@ void YoungList::empty_list() {
   _head = NULL;
   _length = 0;
 
-  empty_list(_survivor_head);
+  if (survivor_length() > 0) {
-  _survivor_head = NULL;
+    empty_list(_survivor_regions->last());
-  _survivor_tail = NULL;
+  }
-  _survivor_length = 0;
+  _survivor_regions->clear();
 
   assert(check_list_empty(), "just making sure...");
 }
 
+uint YoungList::survivor_length() {
+  return _survivor_regions->length();
+}
+
 bool YoungList::check_list_well_formed() {
   bool ret = true;
 
@@ -145,25 +147,25 @@ YoungList::reset_auxilary_lists() {
   _g1h->g1_policy()->note_start_adding_survivor_regions();
   _g1h->g1_policy()->finished_recalculating_age_indexes(true /* is_survivors */);
 
-  for (HeapRegion* curr = _survivor_head;
+  HeapRegion* last = NULL;
-       curr != NULL;
+  for (GrowableArrayIterator<HeapRegion*> it = _survivor_regions->begin();
-       curr = curr->get_next_young_region()) {
+       it != _survivor_regions->end();
+       ++it) {
+    HeapRegion* curr = *it;
     _g1h->g1_policy()->set_region_survivor(curr);
 
     // The region is a non-empty survivor so let's add it to
     // the incremental collection set for the next evacuation
     // pause.
     _g1h->collection_set()->add_survivor_regions(curr);
+
+    curr->set_next_young_region(last);
+    last = curr;
   }
   _g1h->g1_policy()->note_stop_adding_survivor_regions();
 
-  _head   = _survivor_head;
+  _head   = last;
-  _length = _survivor_length;
+  _length = _survivor_regions->length();
-  if (_survivor_head != NULL) {
-    assert(_survivor_tail != NULL, "cause it shouldn't be");
-    assert(_survivor_length > 0, "invariant");
-    _survivor_tail->set_next_young_region(NULL);
-  }
 
   // Don't clear the survivor list handles until the start of
   // the next evacuation pause - we need it in order to re-tag
@@ -174,26 +176,3 @@ YoungList::reset_auxilary_lists() {
 
   assert(check_list_well_formed(), "young list should be well formed");
 }
-
-void YoungList::print() {
-  HeapRegion* lists[] = {_head,   _survivor_head};
-  const char* names[] = {"YOUNG", "SURVIVOR"};
-
-  for (uint list = 0; list < ARRAY_SIZE(lists); ++list) {
-    tty->print_cr("%s LIST CONTENTS", names[list]);
-    HeapRegion *curr = lists[list];
-    if (curr == NULL) {
-      tty->print_cr("  empty");
-    }
-    while (curr != NULL) {
-      tty->print_cr("  " HR_FORMAT ", P: " PTR_FORMAT ", N: " PTR_FORMAT ", age: %4d",
-                             HR_FORMAT_PARAMS(curr),
-                             p2i(curr->prev_top_at_mark_start()),
-                             p2i(curr->next_top_at_mark_start()),
-                             curr->age_in_surv_rate_group_cond());
-      curr = curr->get_next_young_region();
-    }
-  }
-
-  tty->cr();
-}

hotspot/src/share/vm/gc/g1/youngList.hpp

@@ -28,17 +28,17 @@
 #include "memory/allocation.hpp"
 #include "runtime/globals.hpp"
 
+template <typename T>
+class GrowableArray;
+
 class YoungList : public CHeapObj<mtGC> {
 private:
   G1CollectedHeap* _g1h;
+  GrowableArray<HeapRegion*>* _survivor_regions;
 
   HeapRegion* _head;
 
-  HeapRegion* _survivor_head;
-  HeapRegion* _survivor_tail;
-
   uint        _length;
-  uint        _survivor_length;
 
   void         empty_list(HeapRegion* list);
 
@@ -52,7 +52,9 @@ public:
   bool         is_empty() { return _length == 0; }
   uint         length() { return _length; }
   uint         eden_length() { return length() - survivor_length(); }
-  uint         survivor_length() { return _survivor_length; }
+  uint         survivor_length();
+
+  const GrowableArray<HeapRegion*>* survivor_regions() const { return _survivor_regions; }
 
   // Currently we do not keep track of the used byte sum for the
   // young list and the survivors and it'd be quite a lot of work to
@@ -72,14 +74,10 @@ public:
   void clear() { _head = NULL; _length = 0; }
 
   void clear_survivors() {
-    _survivor_head    = NULL;
+    _survivor_regions->clear();
-    _survivor_tail    = NULL;
-    _survivor_length  = 0;
   }
 
   HeapRegion* first_region() { return _head; }
-  HeapRegion* first_survivor_region() { return _survivor_head; }
-  HeapRegion* last_survivor_region() { return _survivor_tail; }
 
   // debugging
   bool          check_list_well_formed();