8234179: Move HeapRegion::_recorded_rs_length/_predicted_elapsed_time_ms into G1CollectionSet

Reviewed-by: sjohanss, lkorinth

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

@@ -66,6 +66,7 @@ G1CollectionSet::G1CollectionSet(G1CollectedHeap* g1h, G1Policy* policy) :
   _recorded_rs_length(0),
   _inc_build_state(Inactive),
   _inc_part_start(0),
+  _inc_collection_set_stats(NULL),
   _inc_bytes_used_before(0),
   _inc_recorded_rs_length(0),
   _inc_recorded_rs_length_diff(0),
@@ -75,6 +76,7 @@ G1CollectionSet::G1CollectionSet(G1CollectedHeap* g1h, G1Policy* policy) :
 
 G1CollectionSet::~G1CollectionSet() {
   FREE_C_HEAP_ARRAY(uint, _collection_set_regions);
+  FREE_C_HEAP_ARRAY(IncCollectionSetRegionStat, _inc_collection_set_stats);
   free_optional_regions();
   clear_candidates();
 }
@@ -97,6 +99,7 @@ void G1CollectionSet::initialize(uint max_region_length) {
   guarantee(_collection_set_regions == NULL, "Must only initialize once.");
   _collection_set_max_length = max_region_length;
   _collection_set_regions = NEW_C_HEAP_ARRAY(uint, max_region_length, mtGC);
+  _inc_collection_set_stats = NEW_C_HEAP_ARRAY(IncCollectionSetRegionStat, max_region_length, mtGC);
 }
 
 void G1CollectionSet::free_optional_regions() {
@@ -145,6 +148,11 @@ void G1CollectionSet::add_optional_region(HeapRegion* hr) {
 void G1CollectionSet::start_incremental_building() {
   assert(_collection_set_cur_length == 0, "Collection set must be empty before starting a new collection set.");
   assert(_inc_build_state == Inactive, "Precondition");
+#ifdef ASSERT
+  for (size_t i = 0; i < _collection_set_max_length; i++) {
+    _inc_collection_set_stats[i].reset();
+  }
+#endif
 
   _inc_bytes_used_before = 0;
 
@@ -238,18 +246,22 @@ void G1CollectionSet::update_young_region_prediction(HeapRegion* hr,
   assert(hr->is_young(), "Precondition");
   assert(!SafepointSynchronize::is_at_safepoint(), "should not be at a safepoint");
 
-  size_t old_rs_length = hr->recorded_rs_length();
+  IncCollectionSetRegionStat* stat = &_inc_collection_set_stats[hr->hrm_index()];
+
+  size_t old_rs_length = stat->_rs_length;
   assert(old_rs_length <= new_rs_length,
          "Remembered set decreased (changed from " SIZE_FORMAT " to " SIZE_FORMAT " region %u type %s)",
          old_rs_length, new_rs_length, hr->hrm_index(), hr->get_short_type_str());
   size_t rs_length_diff = new_rs_length - old_rs_length;
-  hr->set_recorded_rs_length(new_rs_length);
+  stat->_rs_length = new_rs_length;
   _inc_recorded_rs_length_diff += rs_length_diff;
 
-  double old_non_copy_time = hr->predicted_non_copy_time_ms();
+  double old_non_copy_time = stat->_non_copy_time_ms;
+  assert(old_non_copy_time >= 0.0, "Non copy time for region %u not initialized yet, is %.3f", hr->hrm_index(), old_non_copy_time);
   double new_non_copy_time = predict_region_non_copy_time_ms(hr);
   double non_copy_time_ms_diff = new_non_copy_time - old_non_copy_time;
-  hr->set_predicted_non_copy_time_ms(new_non_copy_time);
+
+  stat->_non_copy_time_ms = new_non_copy_time;
   _inc_predicted_non_copy_time_ms_diff += non_copy_time_ms_diff;
 }
 
@@ -275,30 +287,31 @@ void G1CollectionSet::add_young_region_common(HeapRegion* hr) {
 
   if (!_g1h->collector_state()->in_full_gc()) {
     size_t rs_length = hr->rem_set()->occupied();
-    double region_non_copy_time = predict_region_non_copy_time_ms(hr);
+    double non_copy_time = predict_region_non_copy_time_ms(hr);
 
     // Cache the values we have added to the aggregated information
     // in the heap region in case we have to remove this region from
     // the incremental collection set, or it is updated by the
     // rset sampling code
-    hr->set_recorded_rs_length(rs_length);
-    hr->set_predicted_non_copy_time_ms(region_non_copy_time);
+
+    IncCollectionSetRegionStat* stat = &_inc_collection_set_stats[hr->hrm_index()];
+    stat->_rs_length = rs_length;
+    stat->_non_copy_time_ms = non_copy_time;
 
     _inc_recorded_rs_length += rs_length;
-    _inc_predicted_non_copy_time_ms += region_non_copy_time;
+    _inc_predicted_non_copy_time_ms += non_copy_time;
     _inc_bytes_used_before += hr->used();
   }
 
   assert(!hr->in_collection_set(), "invariant");
   _g1h->register_young_region_with_region_attr(hr);
 
-  size_t collection_set_length = _collection_set_cur_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);
+  assert(_collection_set_cur_length < (UINT_MAX - 1),
+         "Collection set is too large with " SIZE_FORMAT " entries", _collection_set_cur_length);
+  hr->set_young_index_in_cset((uint)_collection_set_cur_length + 1);
 
-  _collection_set_regions[collection_set_length] = hr->hrm_index();
+  _collection_set_regions[_collection_set_cur_length] = hr->hrm_index();
   // Concurrent readers must observe the store of the value in the array before an
   // update to the length field.
   OrderAccess::storestore();

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

@@ -174,6 +174,22 @@ class G1CollectionSet {
   CSetBuildType _inc_build_state;
   size_t _inc_part_start;
 
+  // Information about eden regions in the incremental collection set.
+  struct IncCollectionSetRegionStat {
+    // The predicted non-copy time that was added to the total incremental value
+    // for the collection set.
+    double _non_copy_time_ms;
+    // The remembered set length that was added to the total incremental value
+    // for the collection set.
+    size_t _rs_length;
+
+#ifdef ASSERT
+    // Resets members to "uninitialized" values.
+    void reset() { _rs_length = ~(size_t)0; _non_copy_time_ms = -1.0; }
+#endif
+  };
+
+  IncCollectionSetRegionStat* _inc_collection_set_stats;
   // The associated information that is maintained while the incremental
   // collection set is being built with *young* regions. Used to populate
   // the recorded info for the evacuation pause.

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

@@ -137,8 +137,6 @@ void HeapRegion::hr_clear(bool keep_remset, bool clear_space, bool locked) {
 
   _evacuation_failed = false;
   _gc_efficiency = 0.0;
-  _recorded_rs_length = 0;
-  _predicted_non_copy_time_ms = 0.0;
 }
 
 void HeapRegion::clear_cardtable() {
@@ -255,7 +253,6 @@ HeapRegion::HeapRegion(uint hrm_index,
   _prev_marked_bytes(0), _next_marked_bytes(0),
   _young_index_in_cset(-1),
   _surv_rate_group(NULL), _age_index(SurvRateGroup::InvalidAgeIndex), _gc_efficiency(0.0),
-  _recorded_rs_length(0), _predicted_non_copy_time_ms(0),
   _node_index(G1NUMA::UnknownNodeIndex)
 {
   assert(Universe::on_page_boundary(mr.start()) && Universe::on_page_boundary(mr.end()),

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

@@ -254,14 +254,6 @@ private:
   // The calculated GC efficiency of the region.
   double _gc_efficiency;
 
-  // The remembered set length that was added to the total value
-  // for the collection set.
-  size_t _recorded_rs_length;
-
-  // The predicted time without copy time that was added to total value
-  // for the collection set.
-  double _predicted_non_copy_time_ms;
-
   uint _node_index;
 
   void report_region_type_change(G1HeapRegionTraceType::Type to);
@@ -578,17 +570,6 @@ public:
   template <bool is_gc_active, class Closure>
   inline HeapWord* oops_on_memregion_seq_iterate_careful(MemRegion mr, Closure* cl);
 
-  size_t recorded_rs_length() const         { return _recorded_rs_length; }
-  double predicted_non_copy_time_ms() const { return _predicted_non_copy_time_ms; }
-
-  void set_recorded_rs_length(size_t rs_length) {
-    _recorded_rs_length = rs_length;
-  }
-
-  void set_predicted_non_copy_time_ms(double ms) {
-    _predicted_non_copy_time_ms = ms;
-  }
-
   // Routines for managing a list of code roots (attached to the
   // this region's RSet) that point into this heap region.
   void add_strong_code_root(nmethod* nm);