8364414: G1: Use simpler data structure for holding collection set candidates during calculation

Reviewed-by: ayang, iwalulya

Author: Thomas Schatzl

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

@@ -44,12 +44,7 @@ G1CSetCandidateGroup::G1CSetCandidateGroup() :
 
 void G1CSetCandidateGroup::add(G1HeapRegion* hr) {
   G1CollectionSetCandidateInfo c(hr);
-  add(c);
-}
-
-void G1CSetCandidateGroup::add(G1CollectionSetCandidateInfo& hr_info) {
-  G1HeapRegion* hr = hr_info._r;
-  _candidates.append(hr_info);
+  _candidates.append(c);
   hr->install_cset_group(this);
 }
 
@@ -133,31 +128,6 @@ int G1CSetCandidateGroup::compare_gc_efficiency(G1CSetCandidateGroup** gr1, G1CS
   }
 }
 
-int G1CollectionSetCandidateInfo::compare_region_gc_efficiency(G1CollectionSetCandidateInfo* ci1, G1CollectionSetCandidateInfo* ci2) {
-  // Make sure that null entries are moved to the end.
-  if (ci1->_r == nullptr) {
-    if (ci2->_r == nullptr) {
-      return 0;
-    } else {
-      return 1;
-    }
-  } else if (ci2->_r == nullptr) {
-    return -1;
-  }
-
-  G1Policy* p = G1CollectedHeap::heap()->policy();
-  double gc_efficiency1 = p->predict_gc_efficiency(ci1->_r);
-  double gc_efficiency2 = p->predict_gc_efficiency(ci2->_r);
-
-  if (gc_efficiency1 > gc_efficiency2) {
-    return -1;
-  } else if (gc_efficiency1 < gc_efficiency2) {
-    return 1;
-  } else {
-    return 0;
-  }
-}
-
 G1CSetCandidateGroupList::G1CSetCandidateGroupList() : _groups(8, mtGC), _num_regions(0) { }
 
 void G1CSetCandidateGroupList::append(G1CSetCandidateGroup* group) {
@@ -279,9 +249,9 @@ void G1CollectionSetCandidates::sort_marking_by_efficiency() {
   _from_marking_groups.verify();
 }
 
-void G1CollectionSetCandidates::set_candidates_from_marking(G1CollectionSetCandidateInfo* candidate_infos,
-                                                            uint num_infos) {
-  if (num_infos == 0) {
+void G1CollectionSetCandidates::set_candidates_from_marking(G1HeapRegion** candidates,
+                                                            uint num_candidates) {
+  if (num_candidates == 0) {
     log_debug(gc, ergo, cset) ("No regions selected from marking.");
     return;
   }
@@ -294,7 +264,7 @@ void G1CollectionSetCandidates::set_candidates_from_marking(G1CollectionSetCandi
   // the G1MixedGCCountTarget. For the first collection in a Mixed GC cycle, we can add all regions
   // required to meet this threshold to the same remset group. We are certain these will be collected in
   // the same MixedGC.
-  uint group_limit = p->calc_min_old_cset_length(num_infos);
+  uint group_limit = p->calc_min_old_cset_length(num_candidates);
 
   uint num_added_to_group = 0;
 
@@ -303,8 +273,8 @@ void G1CollectionSetCandidates::set_candidates_from_marking(G1CollectionSetCandi
 
   current = new G1CSetCandidateGroup();
 
-  for (uint i = 0; i < num_infos; i++) {
-    G1HeapRegion* r = candidate_infos[i]._r;
+  for (uint i = 0; i < num_candidates; i++) {
+    G1HeapRegion* r = candidates[i];
     assert(!contains(r), "must not contain region %u", r->hrm_index());
     _contains_map[r->hrm_index()] = CandidateOrigin::Marking;
 
@@ -318,16 +288,16 @@ void G1CollectionSetCandidates::set_candidates_from_marking(G1CollectionSetCandi
       current = new G1CSetCandidateGroup();
       num_added_to_group = 0;
     }
-    current->add(candidate_infos[i]);
+    current->add(r);
     num_added_to_group++;
   }
 
   _from_marking_groups.append(current);
 
-  assert(_from_marking_groups.num_regions() == num_infos, "Must be!");
+  assert(_from_marking_groups.num_regions() == num_candidates, "Must be!");
 
-  log_debug(gc, ergo, cset) ("Finished creating %u collection groups from %u regions", _from_marking_groups.length(), num_infos);
-  _last_marking_candidates_length = num_infos;
+  log_debug(gc, ergo, cset) ("Finished creating %u collection groups from %u regions", _from_marking_groups.length(), num_candidates);
+  _last_marking_candidates_length = num_candidates;
 
   verify();
 }

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

@@ -48,8 +48,6 @@ struct G1CollectionSetCandidateInfo {
     ++_num_unreclaimed;
     return _num_unreclaimed < G1NumCollectionsKeepPinned;
   }
-
-  static int compare_region_gc_efficiency(G1CollectionSetCandidateInfo* ci1, G1CollectionSetCandidateInfo* ci2);
 };
 
 using G1CSetCandidateGroupIterator = GrowableArrayIterator<G1CollectionSetCandidateInfo>;
@@ -91,7 +89,6 @@ class G1CSetCandidateGroup : public CHeapObj<mtGCCardSet>{
   }
 
   void add(G1HeapRegion* hr);
-  void add(G1CollectionSetCandidateInfo& hr_info);
 
   uint length() const { return (uint)_candidates.length(); }
 
@@ -235,10 +232,10 @@ class G1CollectionSetCandidates : public CHeapObj<mtGC> {
 
   void clear();
 
-  // Merge collection set candidates from marking into the current marking list
+  // Merge collection set candidates from marking into the current marking candidates
   // (which needs to be empty).
-  void set_candidates_from_marking(G1CollectionSetCandidateInfo* candidate_infos,
-                                   uint num_infos);
+  void set_candidates_from_marking(G1HeapRegion** candidates,
+                                   uint num_candidates);
   // The most recent length of the list that had been merged last via
   // set_candidates_from_marking(). Used for calculating minimum collection set
   // regions.

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

@@ -31,30 +31,54 @@
 #include "utilities/quickSort.hpp"
 
 // Determine collection set candidates (from marking): For all regions determine
-// whether they should be a collection set candidate, calculate their efficiency,
-// sort and put them into the candidates.
+// whether they should be a collection set candidate. Calculate their efficiency,
+// sort, and put them into the collection set candidates.
+//
 // Threads calculate the GC efficiency of the regions they get to process, and
 // put them into some work area without sorting. At the end that array is sorted and
 // moved to the destination.
 class G1BuildCandidateRegionsTask : public WorkerTask {
-
-  using CandidateInfo = G1CollectionSetCandidateInfo;
-
   // Work area for building the set of collection set candidates. Contains references
   // to heap regions with their GC efficiencies calculated. To reduce contention
   // on claiming array elements, worker threads claim parts of this array in chunks;
   // Array elements may be null as threads might not get enough regions to fill
   // up their chunks completely.
   // Final sorting will remove them.
   class G1BuildCandidateArray : public StackObj {
-
     uint const _max_size;
     uint const _chunk_size;
 
-    CandidateInfo* _data;
+    G1HeapRegion** _data;
 
     uint volatile _cur_claim_idx;
 
+    static int compare_region_gc_efficiency(G1HeapRegion** rr1, G1HeapRegion** rr2) {
+      G1HeapRegion* r1 = *rr1;
+      G1HeapRegion* r2 = *rr2;
+      // Make sure that null entries are moved to the end.
+      if (r1 == nullptr) {
+        if (r2 == nullptr) {
+          return 0;
+        } else {
+          return 1;
+        }
+      } else if (r2 == nullptr) {
+        return -1;
+      }
+
+      G1Policy* p = G1CollectedHeap::heap()->policy();
+      double gc_efficiency1 = p->predict_gc_efficiency(r1);
+      double gc_efficiency2 = p->predict_gc_efficiency(r2);
+
+      if (gc_efficiency1 > gc_efficiency2) {
+        return -1;
+      } else if (gc_efficiency1 < gc_efficiency2) {
+        return 1;
+      } else {
+        return 0;
+      }
+    }
+
     // Calculates the maximum array size that will be used.
     static uint required_array_size(uint num_regions, uint chunk_size, uint num_workers) {
       uint const max_waste = num_workers * chunk_size;
@@ -68,15 +92,15 @@ class G1BuildCandidateRegionsTask : public WorkerTask {
     G1BuildCandidateArray(uint max_num_regions, uint chunk_size, uint num_workers) :
       _max_size(required_array_size(max_num_regions, chunk_size, num_workers)),
       _chunk_size(chunk_size),
-      _data(NEW_C_HEAP_ARRAY(CandidateInfo, _max_size, mtGC)),
+      _data(NEW_C_HEAP_ARRAY(G1HeapRegion*, _max_size, mtGC)),
       _cur_claim_idx(0) {
       for (uint i = 0; i < _max_size; i++) {
-        _data[i] = CandidateInfo();
+        _data[i] = nullptr;
       }
     }
 
     ~G1BuildCandidateArray() {
-      FREE_C_HEAP_ARRAY(CandidateInfo, _data);
+      FREE_C_HEAP_ARRAY(G1HeapRegion*, _data);
     }
 
     // Claim a new chunk, returning its bounds [from, to[.
@@ -92,24 +116,24 @@ class G1BuildCandidateRegionsTask : public WorkerTask {
     // Set element in array.
     void set(uint idx, G1HeapRegion* hr) {
       assert(idx < _max_size, "Index %u out of bounds %u", idx, _max_size);
-      assert(_data[idx]._r == nullptr, "Value must not have been set.");
-      _data[idx] = CandidateInfo(hr);
+      assert(_data[idx] == nullptr, "Value must not have been set.");
+      _data[idx] = hr;
     }
 
     void sort_by_gc_efficiency() {
       if (_cur_claim_idx == 0) {
         return;
       }
       for (uint i = _cur_claim_idx; i < _max_size; i++) {
-        assert(_data[i]._r == nullptr, "must be");
+        assert(_data[i] == nullptr, "must be");
       }
-      qsort(_data, _cur_claim_idx, sizeof(_data[0]), (_sort_Fn)G1CollectionSetCandidateInfo::compare_region_gc_efficiency);
+      qsort(_data, _cur_claim_idx, sizeof(_data[0]), (_sort_Fn)compare_region_gc_efficiency);
       for (uint i = _cur_claim_idx; i < _max_size; i++) {
-        assert(_data[i]._r == nullptr, "must be");
+        assert(_data[i] == nullptr, "must be");
       }
     }
 
-    CandidateInfo* array() const { return _data; }
+    G1HeapRegion** array() const { return _data; }
   };
 
   // Per-region closure. In addition to determining whether a region should be
@@ -193,7 +217,7 @@ class G1BuildCandidateRegionsTask : public WorkerTask {
   // available (for forward progress in evacuation) or the waste accumulated by the
   // removed regions is above the maximum allowed waste.
   // Updates number of candidates and reclaimable bytes given.
-  void prune(CandidateInfo* data) {
+  void prune(G1HeapRegion** data) {
     G1Policy* p = G1CollectedHeap::heap()->policy();
 
     uint num_candidates = Atomic::load(&_num_regions_added);
@@ -211,7 +235,7 @@ class G1BuildCandidateRegionsTask : public WorkerTask {
     uint max_to_prune = num_candidates - min_old_cset_length;
 
     while (true) {
-      G1HeapRegion* r = data[num_candidates - num_pruned - 1]._r;
+      G1HeapRegion* r = data[num_candidates - num_pruned - 1];
       size_t const reclaimable = r->reclaimable_bytes();
       if (num_pruned >= max_to_prune ||
           wasted_bytes + reclaimable > allowed_waste) {

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

@@ -38,11 +38,11 @@ class WorkerThreads;
 class G1CollectionSetChooser : public AllStatic {
   static uint calculate_work_chunk_size(uint num_workers, uint num_regions);
 
-public:
   static size_t mixed_gc_live_threshold_bytes() {
     return G1HeapRegion::GrainBytes * (size_t)G1MixedGCLiveThresholdPercent / 100;
   }
 
+public:
   static bool region_occupancy_low_enough_for_evac(size_t live_bytes) {
     return live_bytes < mixed_gc_live_threshold_bytes();
   }