# HG changeset patch
# User tschatzl
# Date 1566285738 -7200
# Node ID 854e828d6b5b98a8c7e16b642cb617c4fa1a4fdf
# Parent  aff991f6e64d6fd8b458c30b3286c272869efa0c
8227442: Make young_index_in_cset zero-based
Summary: Avoid unnecessary increment of young_index_in_cset in copy_to_survivor_space.
Reviewed-by: kbarrett, sangheki

diff -r aff991f6e64d -r 854e828d6b5b src/hotspot/share/gc/g1/g1CollectedHeap.cpp
--- a/src/hotspot/share/gc/g1/g1CollectedHeap.cpp	Tue Aug 20 07:47:13 2019 +0200
+++ b/src/hotspot/share/gc/g1/g1CollectedHeap.cpp	Tue Aug 20 09:22:18 2019 +0200
@@ -3988,8 +3988,8 @@
       g1h->clear_region_attr(r);
 
       if (r->is_young()) {
-        assert(r->young_index_in_cset() != -1 && (uint)r->young_index_in_cset() < g1h->collection_set()->young_region_length(),
-               "Young index %d is wrong for region %u of type %s with %u young regions",
+        assert(r->young_index_in_cset() != 0 && (uint)r->young_index_in_cset() <= g1h->collection_set()->young_region_length(),
+               "Young index %u is wrong for region %u of type %s with %u young regions",
                r->young_index_in_cset(),
                r->hrm_index(),
                r->get_type_str(),
@@ -4008,7 +4008,7 @@
                          true  /* locked */);
       } else {
         r->uninstall_surv_rate_group();
-        r->set_young_index_in_cset(-1);
+        r->clear_young_index_in_cset();
         r->set_evacuation_failed(false);
         // When moving a young gen region to old gen, we "allocate" that whole region
         // there. This is in addition to any already evacuated objects. Notify the
@@ -4381,7 +4381,7 @@
   virtual bool do_heap_region(HeapRegion* r) {
     assert(r->in_collection_set(), "Region %u must have been in collection set", r->hrm_index());
     G1CollectedHeap::heap()->clear_region_attr(r);
-    r->set_young_index_in_cset(-1);
+    r->clear_young_index_in_cset();
     return false;
   }
 };
diff -r aff991f6e64d -r 854e828d6b5b src/hotspot/share/gc/g1/g1CollectionSet.cpp
--- a/src/hotspot/share/gc/g1/g1CollectionSet.cpp	Tue Aug 20 07:47:13 2019 +0200
+++ b/src/hotspot/share/gc/g1/g1CollectionSet.cpp	Tue Aug 20 09:22:18 2019 +0200
@@ -279,8 +279,10 @@
   assert(_inc_build_state == Active, "Precondition");
 
   size_t collection_set_length = _collection_set_cur_length;
-  assert(collection_set_length <= INT_MAX, "Collection set is too large with %d entries", (int)collection_set_length);
-  hr->set_young_index_in_cset((int)collection_set_length);
+  // We use UINT_MAX as "invalid" marker in verification.
+  assert(collection_set_length < (UINT_MAX - 1),
+         "Collection set is too large with " SIZE_FORMAT " entries", collection_set_length);
+  hr->set_young_index_in_cset((uint)collection_set_length + 1);
 
   _collection_set_regions[collection_set_length] = hr->hrm_index();
   // Concurrent readers must observe the store of the value in the array before an
@@ -550,12 +552,12 @@
 class G1VerifyYoungCSetIndicesClosure : public HeapRegionClosure {
 private:
   size_t _young_length;
-  int* _heap_region_indices;
+  uint* _heap_region_indices;
 public:
   G1VerifyYoungCSetIndicesClosure(size_t young_length) : HeapRegionClosure(), _young_length(young_length) {
-    _heap_region_indices = NEW_C_HEAP_ARRAY(int, young_length, mtGC);
-    for (size_t i = 0; i < young_length; i++) {
-      _heap_region_indices[i] = -1;
+    _heap_region_indices = NEW_C_HEAP_ARRAY(uint, young_length + 1, mtGC);
+    for (size_t i = 0; i < young_length + 1; i++) {
+      _heap_region_indices[i] = UINT_MAX;
     }
   }
   ~G1VerifyYoungCSetIndicesClosure() {
@@ -563,12 +565,12 @@
   }
 
   virtual bool do_heap_region(HeapRegion* r) {
-    const int idx = r->young_index_in_cset();
+    const uint idx = r->young_index_in_cset();
 
-    assert(idx > -1, "Young index must be set for all regions in the incremental collection set but is not for region %u.", r->hrm_index());
-    assert((size_t)idx < _young_length, "Young cset index too large for region %u", r->hrm_index());
+    assert(idx > 0, "Young index must be set for all regions in the incremental collection set but is not for region %u.", r->hrm_index());
+    assert(idx <= _young_length, "Young cset index %u too large for region %u", idx, r->hrm_index());
 
-    assert(_heap_region_indices[idx] == -1,
+    assert(_heap_region_indices[idx] == UINT_MAX,
            "Index %d used by multiple regions, first use by region %u, second by region %u",
            idx, _heap_region_indices[idx], r->hrm_index());
 
diff -r aff991f6e64d -r 854e828d6b5b src/hotspot/share/gc/g1/g1ParScanThreadState.cpp
--- a/src/hotspot/share/gc/g1/g1ParScanThreadState.cpp	Tue Aug 20 07:47:13 2019 +0200
+++ b/src/hotspot/share/gc/g1/g1ParScanThreadState.cpp	Tue Aug 20 09:22:18 2019 +0200
@@ -59,19 +59,13 @@
     _old_gen_is_full(false),
     _num_optional_regions(optional_cset_length)
 {
-  // we allocate G1YoungSurvRateNumRegions plus one entries, since
-  // we "sacrifice" entry 0 to keep track of surviving bytes for
-  // non-young regions (where the age is -1)
+  // We allocate number of young gen regions in the collection set plus one
+  // entries, since entry 0 keeps track of surviving bytes for non-young regions.
   // We also add a few elements at the beginning and at the end in
   // an attempt to eliminate cache contention
-  size_t real_length = 1 + young_cset_length;
-  size_t array_length = PADDING_ELEM_NUM +
-                      real_length +
-                      PADDING_ELEM_NUM;
+  size_t real_length = young_cset_length + 1;
+  size_t array_length = PADDING_ELEM_NUM + real_length + PADDING_ELEM_NUM;
   _surviving_young_words_base = NEW_C_HEAP_ARRAY(size_t, array_length, mtGC);
-  if (_surviving_young_words_base == NULL)
-    vm_exit_out_of_memory(array_length * sizeof(size_t), OOM_MALLOC_ERROR,
-                          "Not enough space for young surv histo.");
   _surviving_young_words = _surviving_young_words_base + PADDING_ELEM_NUM;
   memset(_surviving_young_words, 0, real_length * sizeof(size_t));
 
@@ -94,9 +88,9 @@
   _plab_allocator->flush_and_retire_stats();
   _g1h->policy()->record_age_table(&_age_table);
 
-  uint length = _g1h->collection_set()->young_region_length();
-  for (uint region_index = 0; region_index < length; region_index++) {
-    surviving_young_words[region_index] += _surviving_young_words[region_index];
+  uint length = _g1h->collection_set()->young_region_length() + 1;
+  for (uint i = 0; i < length; i++) {
+    surviving_young_words[i] += _surviving_young_words[i];
   }
 }
 
@@ -226,11 +220,6 @@
                                                  oop const old,
                                                  markWord const old_mark) {
   const size_t word_sz = old->size();
-  HeapRegion* const from_region = _g1h->heap_region_containing(old);
-  // +1 to make the -1 indexes valid...
-  const int young_index = from_region->young_index_in_cset()+1;
-  assert( (from_region->is_young() && young_index >  0) ||
-         (!from_region->is_young() && young_index == 0), "invariant" );
 
   uint age = 0;
   G1HeapRegionAttr dest_attr = next_region_attr(region_attr, old_mark, age);
@@ -281,6 +270,12 @@
   if (forward_ptr == NULL) {
     Copy::aligned_disjoint_words((HeapWord*) old, obj_ptr, word_sz);
 
+    HeapRegion* const from_region = _g1h->heap_region_containing(old);
+    const uint young_index = from_region->young_index_in_cset();
+
+    assert((from_region->is_young() && young_index >  0) ||
+           (!from_region->is_young() && young_index == 0), "invariant" );
+
     if (dest_attr.is_young()) {
       if (age < markWord::max_age) {
         age++;
@@ -303,7 +298,7 @@
     if (G1StringDedup::is_enabled()) {
       const bool is_from_young = region_attr.is_young();
       const bool is_to_young = dest_attr.is_young();
-      assert(is_from_young == _g1h->heap_region_containing(old)->is_young(),
+      assert(is_from_young == from_region->is_young(),
              "sanity");
       assert(is_to_young == _g1h->heap_region_containing(obj)->is_young(),
              "sanity");
@@ -415,7 +410,7 @@
     _g1h(g1h),
     _rdcqs(rdcqs),
     _states(NEW_C_HEAP_ARRAY(G1ParScanThreadState*, n_workers, mtGC)),
-    _surviving_young_words_total(NEW_C_HEAP_ARRAY(size_t, young_cset_length, mtGC)),
+    _surviving_young_words_total(NEW_C_HEAP_ARRAY(size_t, young_cset_length + 1, mtGC)),
     _young_cset_length(young_cset_length),
     _optional_cset_length(optional_cset_length),
     _n_workers(n_workers),
@@ -423,7 +418,7 @@
   for (uint i = 0; i < n_workers; ++i) {
     _states[i] = NULL;
   }
-  memset(_surviving_young_words_total, 0, young_cset_length * sizeof(size_t));
+  memset(_surviving_young_words_total, 0, (young_cset_length + 1) * sizeof(size_t));
 }
 
 G1ParScanThreadStateSet::~G1ParScanThreadStateSet() {
diff -r aff991f6e64d -r 854e828d6b5b src/hotspot/share/gc/g1/g1ParScanThreadState.hpp
--- a/src/hotspot/share/gc/g1/g1ParScanThreadState.hpp	Tue Aug 20 07:47:13 2019 +0200
+++ b/src/hotspot/share/gc/g1/g1ParScanThreadState.hpp	Tue Aug 20 09:22:18 2019 +0200
@@ -145,12 +145,6 @@
   size_t lab_waste_words() const;
   size_t lab_undo_waste_words() const;
 
-  size_t* surviving_young_words() {
-    // We add one to hide entry 0 which accumulates surviving words for
-    // age -1 regions (i.e. non-young ones)
-    return _surviving_young_words + 1;
-  }
-
   void flush(size_t* surviving_young_words);
 
 private:
diff -r aff991f6e64d -r 854e828d6b5b src/hotspot/share/gc/g1/heapRegion.cpp
--- a/src/hotspot/share/gc/g1/heapRegion.cpp	Tue Aug 20 07:47:13 2019 +0200
+++ b/src/hotspot/share/gc/g1/heapRegion.cpp	Tue Aug 20 09:22:18 2019 +0200
@@ -119,7 +119,7 @@
   assert(!in_collection_set(),
          "Should not clear heap region %u in the collection set", hrm_index());
 
-  set_young_index_in_cset(-1);
+  clear_young_index_in_cset();
   clear_index_in_opt_cset();
   uninstall_surv_rate_group();
   set_free();
diff -r aff991f6e64d -r 854e828d6b5b src/hotspot/share/gc/g1/heapRegion.hpp
--- a/src/hotspot/share/gc/g1/heapRegion.hpp	Tue Aug 20 07:47:13 2019 +0200
+++ b/src/hotspot/share/gc/g1/heapRegion.hpp	Tue Aug 20 09:22:18 2019 +0200
@@ -255,7 +255,9 @@
   // The index in the optional regions array, if this region
   // is considered optional during a mixed collections.
   uint _index_in_opt_cset;
-  int  _young_index_in_cset;
+
+  // Data for young region survivor prediction.
+  uint  _young_index_in_cset;
   SurvRateGroup* _surv_rate_group;
   int  _age_index;
 
@@ -563,21 +565,24 @@
   void set_index_in_opt_cset(uint index) { _index_in_opt_cset = index; }
   void clear_index_in_opt_cset() { _index_in_opt_cset = InvalidCSetIndex; }
 
-  int  young_index_in_cset() const { return _young_index_in_cset; }
-  void set_young_index_in_cset(int index) {
-    assert( (index == -1) || is_young(), "pre-condition" );
+  uint  young_index_in_cset() const { return _young_index_in_cset; }
+  void clear_young_index_in_cset() { _young_index_in_cset = 0; }
+  void set_young_index_in_cset(uint index) {
+    assert(index != UINT_MAX, "just checking");
+    assert(index != 0, "just checking");
+    assert(is_young(), "pre-condition");
     _young_index_in_cset = index;
   }
 
   int age_in_surv_rate_group() {
-    assert( _surv_rate_group != NULL, "pre-condition" );
-    assert( _age_index > -1, "pre-condition" );
+    assert(_surv_rate_group != NULL, "pre-condition");
+    assert(_age_index > -1, "pre-condition");
     return _surv_rate_group->age_in_group(_age_index);
   }
 
   void record_surv_words_in_group(size_t words_survived) {
-    assert( _surv_rate_group != NULL, "pre-condition" );
-    assert( _age_index > -1, "pre-condition" );
+    assert(_surv_rate_group != NULL, "pre-condition");
+    assert(_age_index > -1, "pre-condition");
     int age_in_group = age_in_surv_rate_group();
     _surv_rate_group->record_surviving_words(age_in_group, words_survived);
   }
@@ -594,9 +599,9 @@
   }
 
   void install_surv_rate_group(SurvRateGroup* surv_rate_group) {
-    assert( surv_rate_group != NULL, "pre-condition" );
-    assert( _surv_rate_group == NULL, "pre-condition" );
-    assert( is_young(), "pre-condition" );
+    assert(surv_rate_group != NULL, "pre-condition");
+    assert(_surv_rate_group == NULL, "pre-condition");
+    assert(is_young(), "pre-condition");
 
     _surv_rate_group = surv_rate_group;
     _age_index = surv_rate_group->next_age_index();
@@ -604,13 +609,13 @@
 
   void uninstall_surv_rate_group() {
     if (_surv_rate_group != NULL) {
-      assert( _age_index > -1, "pre-condition" );
-      assert( is_young(), "pre-condition" );
+      assert(_age_index > -1, "pre-condition");
+      assert(is_young(), "pre-condition");
 
       _surv_rate_group = NULL;
       _age_index = -1;
     } else {
-      assert( _age_index == -1, "pre-condition" );
+      assert(_age_index == -1, "pre-condition");
     }
   }