8233588: Clean up SurvRateGroup

Remove unnecessary members, enforce stricter visibility within use.

Reviewed-by: sangheki, kbarrett

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

@@ -330,21 +330,19 @@ void G1CollectionSet::add_eden_region(HeapRegion* hr) {
 class G1VerifyYoungAgesClosure : public HeapRegionClosure {
 public:
   bool _valid;
-public:
+
   G1VerifyYoungAgesClosure() : HeapRegionClosure(), _valid(true) { }
 
   virtual bool do_heap_region(HeapRegion* r) {
     guarantee(r->is_young(), "Region must be young but is %s", r->get_type_str());
 
-    SurvRateGroup* group = r->surv_rate_group();
+    if (!r->has_surv_rate_group()) {
-
+      log_error(gc, verify)("## encountered young region without surv_rate_group");
-    if (group == NULL) {
-      log_error(gc, verify)("## encountered NULL surv_rate_group in young region");
       _valid = false;
     }
 
-    if (r->age_in_surv_rate_group() < 0) {
+    if (!r->has_valid_age_in_surv_rate()) {
-      log_error(gc, verify)("## encountered negative age in young region");
+      log_error(gc, verify)("## encountered invalid age in young region");
       _valid = false;
     }
 
@@ -379,7 +377,7 @@ public:
                   HR_FORMAT_PARAMS(r),
                   p2i(r->prev_top_at_mark_start()),
                   p2i(r->next_top_at_mark_start()),
-                  r->age_in_surv_rate_group_cond());
+                  r->has_surv_rate_group() ? r->age_in_surv_rate_group() : -1);
     return false;
   }
 };

src/hotspot/share/gc/g1/g1Policy.cpp

@@ -911,12 +911,6 @@ void G1Policy::print_phases() {
   phase_times()->print();
 }
 
-double G1Policy::predict_yg_surv_rate(int age, SurvRateGroup* surv_rate_group) const {
-  TruncatedSeq* seq = surv_rate_group->get_seq(age);
-  guarantee(seq->num() > 0, "There should be some young gen survivor samples available. Tried to access with age %d", age);
-  return _predictor.get_new_unit_prediction(seq);
-}
-
 double G1Policy::accum_yg_surv_rate_pred(int age) const {
   return _short_lived_surv_rate_group->accum_surv_rate_pred(age);
 }
@@ -940,10 +934,7 @@ size_t G1Policy::predict_bytes_to_copy(HeapRegion* hr) const {
   if (!hr->is_young()) {
     bytes_to_copy = hr->max_live_bytes();
   } else {
-    assert(hr->age_in_surv_rate_group() != -1, "invariant");
+    bytes_to_copy = (size_t) (hr->used() * hr->surv_rate_prediction(_predictor));
-    int age = hr->age_in_surv_rate_group();
-    double yg_surv_rate = predict_yg_surv_rate(age, hr->surv_rate_group());
-    bytes_to_copy = (size_t) (hr->used() * yg_surv_rate);
   }
   return bytes_to_copy;
 }
@@ -1399,7 +1390,6 @@ void G1Policy::transfer_survivors_to_cset(const G1SurvivorRegions* survivors) {
 
   // Add survivor regions to SurvRateGroup.
   note_start_adding_survivor_regions();
-  finished_recalculating_age_indexes(true /* is_survivors */);
 
   HeapRegion* last = NULL;
   for (GrowableArrayIterator<HeapRegion*> it = survivors->regions()->begin();
@@ -1421,6 +1411,4 @@ void G1Policy::transfer_survivors_to_cset(const G1SurvivorRegions* survivors) {
   // the next evacuation pause - we need it in order to re-tag
   // the survivor regions from this evacuation pause as 'young'
   // at the start of the next.
-
-  finished_recalculating_age_indexes(false /* is_survivors */);
 }

src/hotspot/share/gc/g1/g1Policy.hpp

@@ -167,10 +167,6 @@ public:
     return _mmu_tracker->max_gc_time() * 1000.0;
   }
 
-  double predict_yg_surv_rate(int age, SurvRateGroup* surv_rate_group) const;
-
-  double predict_yg_surv_rate(int age) const;
-
   double accum_yg_surv_rate_pred(int age) const;
 
 private:
@@ -375,14 +371,6 @@ public:
   // the initial-mark work and start a marking cycle.
   void decide_on_conc_mark_initiation();
 
-  void finished_recalculating_age_indexes(bool is_survivors) {
-    if (is_survivors) {
-      _survivor_surv_rate_group->finished_recalculating_age_indexes();
-    } else {
-      _short_lived_surv_rate_group->finished_recalculating_age_indexes();
-    }
-  }
-
   size_t young_list_target_length() const { return _young_list_target_length; }
 
   bool should_allocate_mutator_region() const;

src/hotspot/share/gc/g1/heapRegion.cpp

@@ -256,7 +256,7 @@ HeapRegion::HeapRegion(uint hrm_index,
   _prev_top_at_mark_start(NULL), _next_top_at_mark_start(NULL),
   _prev_marked_bytes(0), _next_marked_bytes(0),
   _young_index_in_cset(-1),
-  _surv_rate_group(NULL), _age_index(-1), _gc_efficiency(0.0),
+  _surv_rate_group(NULL), _age_index(SurvRateGroup::InvalidAgeIndex), _gc_efficiency(0.0),
   _recorded_rs_length(0), _predicted_elapsed_time_ms(0),
   _node_index(G1NUMA::UnknownNodeIndex)
 {

src/hotspot/share/gc/g1/heapRegion.hpp

@@ -38,6 +38,7 @@
 
 class G1CollectedHeap;
 class G1CMBitMap;
+class G1Predictions;
 class HeapRegionRemSet;
 class HeapRegion;
 class HeapRegionSetBase;
@@ -544,50 +545,17 @@ public:
     _young_index_in_cset = index;
   }
 
-  int age_in_surv_rate_group() {
+  int age_in_surv_rate_group() const;
-    assert(_surv_rate_group != NULL, "pre-condition");
+  bool has_valid_age_in_surv_rate() const;
-    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) {
+  bool has_surv_rate_group() const;
-    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);
-  }
 
-  int age_in_surv_rate_group_cond() {
+  double surv_rate_prediction(G1Predictions const& predictor) const;
-    if (_surv_rate_group != NULL)
-      return age_in_surv_rate_group();
-    else
-      return -1;
-  }
 
-  SurvRateGroup* surv_rate_group() {
+  void install_surv_rate_group(SurvRateGroup* surv_rate_group);
-    return _surv_rate_group;
+  void uninstall_surv_rate_group();
-  }
 
-  void install_surv_rate_group(SurvRateGroup* surv_rate_group) {
+  void record_surv_words_in_group(size_t words_survived);
-    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();
-  }
-
-  void uninstall_surv_rate_group() {
-    if (_surv_rate_group != NULL) {
-      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");
-    }
-  }
 
   // Determine if an object has been allocated since the last
   // mark performed by the collector. This returns true iff the object

src/hotspot/share/gc/g1/heapRegion.inline.hpp

@@ -28,6 +28,7 @@
 #include "gc/g1/g1BlockOffsetTable.inline.hpp"
 #include "gc/g1/g1CollectedHeap.inline.hpp"
 #include "gc/g1/g1ConcurrentMarkBitMap.inline.hpp"
+#include "gc/g1/g1Predictions.hpp"
 #include "gc/g1/heapRegion.hpp"
 #include "oops/oop.inline.hpp"
 #include "runtime/atomic.hpp"
@@ -370,4 +371,51 @@ HeapWord* HeapRegion::oops_on_memregion_seq_iterate_careful(MemRegion mr,
   }
 }
 
+inline int HeapRegion::age_in_surv_rate_group() const {
+  assert(has_surv_rate_group(), "pre-condition");
+  assert(has_valid_age_in_surv_rate(), "pre-condition");
+  return _surv_rate_group->age_in_group(_age_index);
+}
+
+inline bool HeapRegion::has_valid_age_in_surv_rate() const {
+  return SurvRateGroup::is_valid_age_index(_age_index);
+}
+
+inline bool HeapRegion::has_surv_rate_group() const {
+  return _surv_rate_group != NULL;
+}
+
+inline double HeapRegion::surv_rate_prediction(G1Predictions const& predictor) const {
+  assert(has_surv_rate_group(), "pre-condition");
+  return _surv_rate_group->surv_rate_pred(predictor, age_in_surv_rate_group());
+}
+
+inline void HeapRegion::install_surv_rate_group(SurvRateGroup* surv_rate_group) {
+  assert(surv_rate_group != NULL, "pre-condition");
+  assert(!has_surv_rate_group(), "pre-condition");
+  assert(is_young(), "pre-condition");
+
+  _surv_rate_group = surv_rate_group;
+  _age_index = surv_rate_group->next_age_index();
+}
+
+inline void HeapRegion::uninstall_surv_rate_group() {
+  if (has_surv_rate_group()) {
+    assert(has_valid_age_in_surv_rate(), "pre-condition");
+    assert(is_young(), "pre-condition");
+
+    _surv_rate_group = NULL;
+    _age_index = SurvRateGroup::InvalidAgeIndex;
+  } else {
+    assert(!has_valid_age_in_surv_rate(), "pre-condition");
+  }
+}
+
+inline void HeapRegion::record_surv_words_in_group(size_t words_survived) {
+  assert(has_surv_rate_group(), "pre-condition");
+  assert(has_valid_age_in_surv_rate(), "pre-condition");
+  int age_in_group = age_in_surv_rate_group();
+  _surv_rate_group->record_surviving_words(age_in_group, words_survived);
+}
+
 #endif // SHARE_GC_G1_HEAPREGION_INLINE_HPP

src/hotspot/share/gc/g1/survRateGroup.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2001, 2015, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2001, 2019, 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
@@ -34,66 +34,60 @@ SurvRateGroup::SurvRateGroup() :
   _stats_arrays_length(0),
   _accum_surv_rate_pred(NULL),
   _last_pred(0.0),
-  _surv_rate_pred(NULL),
+  _surv_rate_predictors(NULL),
-  _all_regions_allocated(0),
+  _num_added_regions(0) {
-  _region_num(0),
-  _setup_seq_num(0)
-{
   reset();
   start_adding_regions();
 }
 
 void SurvRateGroup::reset() {
-  _all_regions_allocated = 0;
-  _setup_seq_num         = 0;
   _last_pred = 0.0;
   // the following will set up the arrays with length 1
-  _region_num            = 1;
+  _num_added_regions = 1;
 
   // The call to stop_adding_regions() will use "new" to refill
   // the _surv_rate_pred array, so we need to make sure to call
   // "delete".
   for (size_t i = 0; i < _stats_arrays_length; ++i) {
-    delete _surv_rate_pred[i];
+    delete _surv_rate_predictors[i];
   }
   _stats_arrays_length = 0;
 
   stop_adding_regions();
 
   // Seed initial _surv_rate_pred and _accum_surv_rate_pred values
-  guarantee( _stats_arrays_length == 1, "invariant" );
+  guarantee(_stats_arrays_length == 1, "invariant" );
-  guarantee( _surv_rate_pred[0] != NULL, "invariant" );
+  guarantee(_surv_rate_predictors[0] != NULL, "invariant" );
   const double initial_surv_rate = 0.4;
-  _surv_rate_pred[0]->add(initial_surv_rate);
+  _surv_rate_predictors[0]->add(initial_surv_rate);
   _last_pred = _accum_surv_rate_pred[0] = initial_surv_rate;
 
-  _region_num = 0;
+  _num_added_regions = 0;
 }
 
 void SurvRateGroup::start_adding_regions() {
-  _setup_seq_num   = _stats_arrays_length;
+  _num_added_regions = 0;
-  _region_num      = 0;
 }
 
 void SurvRateGroup::stop_adding_regions() {
-  if (_region_num > _stats_arrays_length) {
+  if (_num_added_regions > _stats_arrays_length) {
-    _accum_surv_rate_pred = REALLOC_C_HEAP_ARRAY(double, _accum_surv_rate_pred, _region_num, mtGC);
+    _accum_surv_rate_pred = REALLOC_C_HEAP_ARRAY(double, _accum_surv_rate_pred, _num_added_regions, mtGC);
-    _surv_rate_pred = REALLOC_C_HEAP_ARRAY(TruncatedSeq*, _surv_rate_pred, _region_num, mtGC);
+    _surv_rate_predictors = REALLOC_C_HEAP_ARRAY(TruncatedSeq*, _surv_rate_predictors, _num_added_regions, mtGC);
 
-    for (size_t i = _stats_arrays_length; i < _region_num; ++i) {
+    for (size_t i = _stats_arrays_length; i < _num_added_regions; ++i) {
-      _surv_rate_pred[i] = new TruncatedSeq(10);
+      _surv_rate_predictors[i] = new TruncatedSeq(10);
     }
 
-    _stats_arrays_length = _region_num;
+    _stats_arrays_length = _num_added_regions;
   }
 }
 
 void SurvRateGroup::record_surviving_words(int age_in_group, size_t surv_words) {
-  guarantee( 0 <= age_in_group && (size_t) age_in_group < _region_num,
+  guarantee(0 <= age_in_group && (size_t)age_in_group < _num_added_regions,
-             "pre-condition" );
+            "age_in_group is %d not between 0 and " SIZE_FORMAT, age_in_group, _num_added_regions);
 
-  double surv_rate = (double) surv_words / (double) HeapRegion::GrainWords;
+  double surv_rate = (double)surv_words / HeapRegion::GrainWords;
-  _surv_rate_pred[age_in_group]->add(surv_rate);
+  _surv_rate_predictors[age_in_group]->add(surv_rate);
 }
 
 void SurvRateGroup::all_surviving_words_recorded(const G1Predictions& predictor, bool update_predictors) {
@@ -104,10 +98,10 @@ void SurvRateGroup::all_surviving_words_recorded(const G1Predictions& predictor,
 }
 
 void SurvRateGroup::fill_in_last_surv_rates() {
-  if (_region_num > 0) { // conservative
+  if (_num_added_regions > 0) { // conservative
-    double surv_rate = _surv_rate_pred[_region_num-1]->last();
+    double surv_rate = _surv_rate_predictors[_num_added_regions-1]->last();
-    for (size_t i = _region_num; i < _stats_arrays_length; ++i) {
+    for (size_t i = _num_added_regions; i < _stats_arrays_length; ++i) {
-      _surv_rate_pred[i]->add(surv_rate);
+      _surv_rate_predictors[i]->add(surv_rate);
     }
   }
 }
@@ -116,7 +110,7 @@ void SurvRateGroup::finalize_predictions(const G1Predictions& predictor) {
   double accum = 0.0;
   double pred = 0.0;
   for (size_t i = 0; i < _stats_arrays_length; ++i) {
-    pred = predictor.get_new_unit_prediction(_surv_rate_pred[i]);
+    pred = predictor.get_new_unit_prediction(_surv_rate_predictors[i]);
     accum += pred;
     _accum_surv_rate_pred[i] = accum;
   }

src/hotspot/share/gc/g1/survRateGroup.hpp

@@ -25,24 +25,24 @@
 #ifndef SHARE_GC_G1_SURVRATEGROUP_HPP
 #define SHARE_GC_G1_SURVRATEGROUP_HPP
 
+#include "gc/g1/g1Predictions.hpp"
 #include "utilities/numberSeq.hpp"
 
-class G1Predictions;
-
 class SurvRateGroup : public CHeapObj<mtGC> {
-private:
   size_t  _stats_arrays_length;
   double* _accum_surv_rate_pred;
   double  _last_pred;
-  TruncatedSeq** _surv_rate_pred;
+  TruncatedSeq** _surv_rate_predictors;
 
-  int _all_regions_allocated;
+  size_t _num_added_regions;   // The number of regions in this SurvRateGroup
-  size_t _region_num;
-  size_t _setup_seq_num;
 
   void fill_in_last_surv_rates();
   void finalize_predictions(const G1Predictions& predictor);
+
 public:
+  static const int InvalidAgeIndex = -1;
+  static bool is_valid_age_index(int age) { return age >= 0; }
+
   SurvRateGroup();
   void reset();
   void start_adding_regions();
@@ -50,42 +50,34 @@ public:
   void record_surviving_words(int age_in_group, size_t surv_words);
   void all_surviving_words_recorded(const G1Predictions& predictor, bool update_predictors);
 
-  size_t region_num() const { return _region_num; }
-
   double accum_surv_rate_pred(int age) const {
-    assert(age >= 0, "must be");
+    assert(_stats_arrays_length > 0, "invariant" );
+    assert(is_valid_age_index(age), "must be");
     if ((size_t)age < _stats_arrays_length)
       return _accum_surv_rate_pred[age];
     else {
-      double diff = (double) (age - _stats_arrays_length + 1);
+      double diff = (double)(age - _stats_arrays_length + 1);
-      return _accum_surv_rate_pred[_stats_arrays_length-1] + diff * _last_pred;
+      return _accum_surv_rate_pred[_stats_arrays_length - 1] + diff * _last_pred;
     }
   }
 
-  TruncatedSeq* get_seq(size_t age) const {
+  double surv_rate_pred(G1Predictions const& predictor, int age) const {
-    if (age >= _setup_seq_num) {
+    assert(is_valid_age_index(age), "must be");
-      guarantee( _setup_seq_num > 0, "invariant" );
+
-      age = _setup_seq_num-1;
+    age = MIN2(age, (int)_stats_arrays_length - 1);
-    }
+
-    TruncatedSeq* seq = _surv_rate_pred[age];
+    return predictor.get_new_unit_prediction(_surv_rate_predictors[age]);
-    guarantee( seq != NULL, "invariant" );
-    return seq;
   }
 
   int next_age_index() {
-    ++_region_num;
+    return (int)++_num_added_regions;
-    return (int) ++_all_regions_allocated;
   }
 
   int age_in_group(int age_index) const {
-    int ret = (int) (_all_regions_allocated - age_index);
+    int result = (int)(_num_added_regions - age_index);
-    assert( ret >= 0, "invariant" );
+    assert(is_valid_age_index(result), "invariant" );
-    return ret;
+    return result;
   }
-  void finished_recalculating_age_indexes() {
-    _all_regions_allocated = 0;
-  }
-
 };
 
 #endif // SHARE_GC_G1_SURVRATEGROUP_HPP