8331048: G1: Prune rebuild candidates based on G1HeapWastePercent early

Reviewed-by: ayang, tschatzl

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

@@ -23,7 +23,7 @@
  */
 
 #include "precompiled.hpp"
-#include "gc/g1/g1CollectionSetCandidates.hpp"
+#include "gc/g1/g1CollectionSetCandidates.inline.hpp"
 #include "gc/g1/g1CollectionSetChooser.hpp"
 #include "gc/g1/g1HeapRegion.inline.hpp"
 #include "utilities/bitMap.inline.hpp"
@@ -44,7 +44,7 @@ void G1CollectionCandidateList::append_unsorted(HeapRegion* r) {
 }
 
 void G1CollectionCandidateList::sort_by_efficiency() {
-  _candidates.sort(compare);
+  _candidates.sort(compare_gc_efficiency);
 }
 
 void G1CollectionCandidateList::remove(G1CollectionCandidateRegionList* other) {
@@ -94,7 +94,22 @@ void G1CollectionCandidateList::verify() {
 }
 #endif
 
-int G1CollectionCandidateList::compare(G1CollectionSetCandidateInfo* ci1, G1CollectionSetCandidateInfo* ci2) {
+int G1CollectionCandidateList::compare_gc_efficiency(G1CollectionSetCandidateInfo* ci1, G1CollectionSetCandidateInfo* ci2) {
+  assert(ci1->_r != nullptr && ci2->_r != nullptr, "Should not be!");
+
+  double gc_eff1 = ci1->_gc_efficiency;
+  double gc_eff2 = ci2->_gc_efficiency;
+
+  if (gc_eff1 > gc_eff2) {
+    return -1;
+  } else if (gc_eff1 < gc_eff2) {
+    return 1;
+  } else {
+    return 0;
+  }
+}
+
+int G1CollectionCandidateList::compare_reclaimble_bytes(G1CollectionSetCandidateInfo* ci1, G1CollectionSetCandidateInfo* ci2) {
   // Make sure that null entries are moved to the end.
   if (ci1->_r == nullptr) {
     if (ci2->_r == nullptr) {
@@ -106,12 +121,12 @@ int G1CollectionCandidateList::compare(G1CollectionSetCandidateInfo* ci1, G1Coll
     return -1;
   }
 
-  double gc_eff1 = ci1->_gc_efficiency;
-  double gc_eff2 = ci2->_gc_efficiency;
+  size_t reclaimable1 = ci1->_r->reclaimable_bytes();
+  size_t reclaimable2 = ci2->_r->reclaimable_bytes();
 
-  if (gc_eff1 > gc_eff2) {
+  if (reclaimable1 > reclaimable2) {
     return -1;
-  } if (gc_eff1 < gc_eff2) {
+  } else if (reclaimable1 < reclaimable2) {
     return 1;
   } else {
     return 0;
@@ -182,6 +197,17 @@ void G1CollectionSetCandidates::clear() {
   _last_marking_candidates_length = 0;
 }
 
+void G1CollectionSetCandidates::sort_marking_by_efficiency() {
+  G1CollectionCandidateListIterator iter = _marking_regions.begin();
+  for (; iter != _marking_regions.end(); ++iter) {
+    HeapRegion* hr = (*iter)->_r;
+    (*iter)->_gc_efficiency = hr->calc_gc_efficiency();
+  }
+  _marking_regions.sort_by_efficiency();
+
+  _marking_regions.verify();
+}
+
 void G1CollectionSetCandidates::set_candidates_from_marking(G1CollectionSetCandidateInfo* candidate_infos,
                                                             uint num_infos) {
   assert(_marking_regions.length() == 0, "must be empty before adding new ones");

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

@@ -129,7 +129,9 @@ class G1CollectionCandidateList : public CHeapObj<mtGC> {
   // Comparison function to order regions in decreasing GC efficiency order. This
   // will cause regions with a lot of live objects and large remembered sets to end
   // up at the end of the list.
-  static int compare(G1CollectionSetCandidateInfo* ci1, G1CollectionSetCandidateInfo* ci2);
+  static int compare_gc_efficiency(G1CollectionSetCandidateInfo* ci1, G1CollectionSetCandidateInfo* ci2);
+
+  static int compare_reclaimble_bytes(G1CollectionSetCandidateInfo* ci1, G1CollectionSetCandidateInfo* ci2);
 
   G1CollectionCandidateListIterator begin() {
     return G1CollectionCandidateListIterator(this, 0);
@@ -213,6 +215,8 @@ class G1CollectionSetCandidates : public CHeapObj<mtGC> {
 
   void sort_by_efficiency();
 
+  void sort_marking_by_efficiency();
+
   // Add the given region to the set of retained regions without regards to the
   // gc efficiency sorting. The retained regions must be re-sorted manually later.
   void add_retained_region_unsorted(HeapRegion* r);

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

@@ -94,17 +94,17 @@ class G1BuildCandidateRegionsTask : public WorkerTask {
     void set(uint idx, HeapRegion* 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, hr->calc_gc_efficiency());
+      _data[idx] = CandidateInfo(hr, 0.0);
     }
 
-    void sort_by_efficiency() {
+    void sort_by_reclaimable_bytes() {
       if (_cur_claim_idx == 0) {
         return;
       }
       for (uint i = _cur_claim_idx; i < _max_size; i++) {
         assert(_data[i]._r == nullptr, "must be");
       }
-      qsort(_data, _cur_claim_idx, sizeof(_data[0]), (_sort_Fn)G1CollectionCandidateList::compare);
+      qsort(_data, _cur_claim_idx, sizeof(_data[0]), (_sort_Fn)G1CollectionCandidateList::compare_reclaimble_bytes);
       for (uint i = _cur_claim_idx; i < _max_size; i++) {
         assert(_data[i]._r == nullptr, "must be");
       }
@@ -152,8 +152,7 @@ class G1BuildCandidateRegionsTask : public WorkerTask {
       }
 
       // Can not add a region without a remembered set to the candidates.
-      assert(!r->rem_set()->is_updating(), "must be");
-      if (!r->rem_set()->is_complete()) {
+      if (!r->rem_set()->is_tracked()) {
         return false;
       }
 
@@ -249,7 +248,7 @@ class G1BuildCandidateRegionsTask : public WorkerTask {
   }
 
   void sort_and_prune_into(G1CollectionSetCandidates* candidates) {
-    _result.sort_by_efficiency();
+    _result.sort_by_reclaimable_bytes();
     prune(_result.array());
     candidates->set_candidates_from_marking(_result.array(),
                                             _num_regions_added);

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

@@ -1438,7 +1438,14 @@ void G1ConcurrentMark::remark() {
 
       log_debug(gc, remset, tracking)("Remembered Set Tracking update regions total %u, selected %u",
                                         _g1h->num_regions(), cl.total_selected_for_rebuild());
+
       _needs_remembered_set_rebuild = (cl.total_selected_for_rebuild() > 0);
+
+      if (_needs_remembered_set_rebuild) {
+        // Prune rebuild candidates based on G1HeapWastePercent.
+        // Improves rebuild time in addition to remembered set memory usage.
+        G1CollectionSetChooser::build(_g1h->workers(), _g1h->num_regions(), _g1h->policy()->candidates());
+      }
     }
 
     if (log_is_enabled(Trace, gc, liveness)) {

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

@@ -1310,7 +1310,7 @@ void G1Policy::decide_on_concurrent_start_pause() {
 void G1Policy::record_concurrent_mark_cleanup_end(bool has_rebuilt_remembered_sets) {
   bool mixed_gc_pending = false;
   if (has_rebuilt_remembered_sets) {
-    G1CollectionSetChooser::build(_g1h->workers(), _g1h->num_regions(), candidates());
+    candidates()->sort_marking_by_efficiency();
     mixed_gc_pending = next_gc_should_be_mixed();
   }
 

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

@@ -184,9 +184,10 @@ class G1Policy: public CHeapObj<mtGC> {
     return _mmu_tracker->max_gc_time() * 1000.0;
   }
 
+  G1CollectionSetCandidates* candidates() const;
+
 private:
   G1CollectionSet* _collection_set;
-  G1CollectionSetCandidates* candidates() const;
 
   double average_time_ms(G1GCPhaseTimes::GCParPhases phase) const;
   double other_time_ms(double pause_time_ms) const;