# HG changeset patch
# User mgerdin
# Date 1461749116 -7200
# Node ID 6503703df0584ef9b1084abf024c3b70e1b4811d
# Parent  95c7e9d6747c45c396fca72a255bb3a9dff54808
8155209: Move setting of young index in cset to G1CollectionSet
Reviewed-by: sjohanss, ehelin

diff -r 95c7e9d6747c -r 6503703df058 hotspot/src/share/vm/gc/g1/g1CollectionSet.cpp
--- a/hotspot/src/share/vm/gc/g1/g1CollectionSet.cpp	Wed Apr 27 11:40:43 2016 -0700
+++ b/hotspot/src/share/vm/gc/g1/g1CollectionSet.cpp	Wed Apr 27 11:25:16 2016 +0200
@@ -48,7 +48,6 @@
   return _policy->predict_region_elapsed_time_ms(hr, collector_state()->gcs_are_young());
 }
 
-
 G1CollectionSet::G1CollectionSet(G1CollectedHeap* g1h, G1Policy* policy) :
   _g1(g1h),
   _policy(policy),
@@ -68,7 +67,8 @@
   _inc_recorded_rs_lengths(0),
   _inc_recorded_rs_lengths_diffs(0),
   _inc_predicted_elapsed_time_ms(0.0),
-  _inc_predicted_elapsed_time_ms_diffs(0.0) {}
+  _inc_predicted_elapsed_time_ms_diffs(0.0),
+  _inc_region_length(0) {}
 
 G1CollectionSet::~G1CollectionSet() {
   delete _cset_chooser;
@@ -78,6 +78,9 @@
                                           uint survivor_cset_region_length) {
   _eden_region_length     = eden_cset_region_length;
   _survivor_region_length = survivor_cset_region_length;
+
+  assert(young_region_length() == _inc_region_length, "should match %u == %u", young_region_length(), _inc_region_length);
+
   _old_region_length      = 0;
 }
 
@@ -107,6 +110,7 @@
   _inc_head = NULL;
   _inc_tail = NULL;
   _inc_bytes_used_before = 0;
+  _inc_region_length = 0;
 
   _inc_recorded_rs_lengths = 0;
   _inc_recorded_rs_lengths_diffs = 0;
@@ -177,9 +181,11 @@
 
 void G1CollectionSet::add_young_region_common(HeapRegion* hr) {
   assert(hr->is_young(), "invariant");
-  assert(hr->young_index_in_cset() > -1, "should have already been set");
   assert(_inc_build_state == Active, "Precondition");
 
+  hr->set_young_index_in_cset(_inc_region_length);
+  _inc_region_length++;
+
   // This routine is used when:
   // * adding survivor regions to the incremental cset at the end of an
   //   evacuation pause or
@@ -306,6 +312,8 @@
     hr = hr->get_next_young_region();
   }
 
+  verify_young_cset_indices();
+
   // Clear the fields that point to the survivor list - they are all young now.
   young_list->clear_survivors();
 
@@ -424,3 +432,25 @@
   double non_young_end_time_sec = os::elapsedTime();
   phase_times()->record_non_young_cset_choice_time_ms((non_young_end_time_sec - non_young_start_time_sec) * 1000.0);
 }
+
+#ifdef ASSERT
+void G1CollectionSet::verify_young_cset_indices() const {
+  ResourceMark rm;
+  uint* heap_region_indices = NEW_RESOURCE_ARRAY(uint, young_region_length());
+  for (uint i = 0; i < young_region_length(); ++i) {
+    heap_region_indices[i] = (uint)-1;
+  }
+
+  for (HeapRegion* hr = _inc_head; hr != NULL; hr = hr->next_in_collection_set()) {
+    const int idx = hr->young_index_in_cset();
+    assert(idx > -1, "must be set for all inc cset regions");
+    assert((uint)idx < young_region_length(), "young cset index too large");
+
+    assert(heap_region_indices[idx] == (uint)-1,
+           "index %d used by multiple regions, first use by %u, second by %u",
+           idx, heap_region_indices[idx], hr->hrm_index());
+
+    heap_region_indices[idx] = hr->hrm_index();
+  }
+}
+#endif
diff -r 95c7e9d6747c -r 6503703df058 hotspot/src/share/vm/gc/g1/g1CollectionSet.hpp
--- a/hotspot/src/share/vm/gc/g1/g1CollectionSet.hpp	Wed Apr 27 11:40:43 2016 -0700
+++ b/hotspot/src/share/vm/gc/g1/g1CollectionSet.hpp	Wed Apr 27 11:25:16 2016 +0200
@@ -104,11 +104,14 @@
   // See the comment for _inc_recorded_rs_lengths_diffs.
   double _inc_predicted_elapsed_time_ms_diffs;
 
+  uint _inc_region_length;
+
   G1CollectorState* collector_state();
   G1GCPhaseTimes* phase_times();
 
   double predict_region_elapsed_time_ms(HeapRegion* hr);
 
+  void verify_young_cset_indices() const NOT_DEBUG_RETURN;
 public:
   G1CollectionSet(G1CollectedHeap* g1h, G1Policy* policy);
   ~G1CollectionSet();
@@ -147,6 +150,7 @@
   void clear_incremental() {
     _inc_head = NULL;
     _inc_tail = NULL;
+    _inc_region_length = 0;
   }
 
   // Stop adding regions to the incremental collection set
diff -r 95c7e9d6747c -r 6503703df058 hotspot/src/share/vm/gc/g1/g1DefaultPolicy.hpp
--- a/hotspot/src/share/vm/gc/g1/g1DefaultPolicy.hpp	Wed Apr 27 11:40:43 2016 -0700
+++ b/hotspot/src/share/vm/gc/g1/g1DefaultPolicy.hpp	Wed Apr 27 11:25:16 2016 +0200
@@ -113,16 +113,14 @@
 
   // Accessors
 
-  void set_region_eden(HeapRegion* hr, int young_index_in_cset) {
+  void set_region_eden(HeapRegion* hr) {
     hr->set_eden();
     hr->install_surv_rate_group(_short_lived_surv_rate_group);
-    hr->set_young_index_in_cset(young_index_in_cset);
   }
 
-  void set_region_survivor(HeapRegion* hr, int young_index_in_cset) {
+  void set_region_survivor(HeapRegion* hr) {
     assert(hr->is_survivor(), "pre-condition");
     hr->install_surv_rate_group(_survivor_surv_rate_group);
-    hr->set_young_index_in_cset(young_index_in_cset);
   }
 
 #ifndef PRODUCT
diff -r 95c7e9d6747c -r 6503703df058 hotspot/src/share/vm/gc/g1/g1Policy.hpp
--- a/hotspot/src/share/vm/gc/g1/g1Policy.hpp	Wed Apr 27 11:40:43 2016 -0700
+++ b/hotspot/src/share/vm/gc/g1/g1Policy.hpp	Wed Apr 27 11:25:16 2016 +0200
@@ -57,8 +57,8 @@
 
   // Accessors
 
-  virtual void set_region_eden(HeapRegion* hr, int young_index_in_cset) = 0;
-  virtual void set_region_survivor(HeapRegion* hr, int young_index_in_cset) = 0;
+  virtual void set_region_eden(HeapRegion* hr) = 0;
+  virtual void set_region_survivor(HeapRegion* hr) = 0;
 
   virtual void record_max_rs_lengths(size_t rs_lengths) = 0;
 
diff -r 95c7e9d6747c -r 6503703df058 hotspot/src/share/vm/gc/g1/youngList.cpp
--- a/hotspot/src/share/vm/gc/g1/youngList.cpp	Wed Apr 27 11:40:43 2016 -0700
+++ b/hotspot/src/share/vm/gc/g1/youngList.cpp	Wed Apr 27 11:25:16 2016 +0200
@@ -46,7 +46,7 @@
   hr->set_next_young_region(_head);
   _head = hr;
 
-  _g1h->g1_policy()->set_region_eden(hr, (int) _length);
+  _g1h->g1_policy()->set_region_eden(hr);
   ++_length;
 }
 
@@ -145,19 +145,16 @@
   _g1h->g1_policy()->note_start_adding_survivor_regions();
   _g1h->g1_policy()->finished_recalculating_age_indexes(true /* is_survivors */);
 
-  int young_index_in_cset = 0;
   for (HeapRegion* curr = _survivor_head;
        curr != NULL;
        curr = curr->get_next_young_region()) {
-    _g1h->g1_policy()->set_region_survivor(curr, young_index_in_cset);
+    _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);
-    young_index_in_cset += 1;
   }
-  assert((uint) young_index_in_cset == _survivor_length, "post-condition");
   _g1h->g1_policy()->note_stop_adding_survivor_regions();
 
   _head   = _survivor_head;