8222492: G1 unnecessarily scans remembered set cards for regions that already have been evacuated

Filter out cards from the current collection set during evacuation increments.

Reviewed-by: kbarrett, sangheki

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

@@ -1111,7 +1111,7 @@ public:
 
  public:
 
-  inline G1HeapRegionAttr region_attr(const oop obj);
+  inline G1HeapRegionAttr region_attr(const void* obj);
 
   // Return "TRUE" iff the given object address is in the reserved
   // region of g1.

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

@@ -29,6 +29,7 @@
 #include "gc/g1/g1CollectedHeap.hpp"
 #include "gc/g1/g1CollectorState.hpp"
 #include "gc/g1/g1Policy.hpp"
+#include "gc/g1/g1RemSet.hpp"
 #include "gc/g1/heapRegionManager.inline.hpp"
 #include "gc/g1/heapRegionRemSet.hpp"
 #include "gc/g1/heapRegionSet.inline.hpp"
@@ -162,8 +163,8 @@ bool G1CollectedHeap::is_in_cset_or_humongous(const oop obj) {
   return _region_attr.is_in_cset_or_humongous((HeapWord*)obj);
 }
 
-G1HeapRegionAttr G1CollectedHeap::region_attr(const oop obj) {
-  return _region_attr.at((HeapWord*)obj);
+G1HeapRegionAttr G1CollectedHeap::region_attr(const void* addr) {
+  return _region_attr.at((HeapWord*)addr);
 }
 
 void G1CollectedHeap::register_humongous_region_with_region_attr(uint index) {
@@ -176,6 +177,7 @@ void G1CollectedHeap::register_region_with_region_attr(HeapRegion* r) {
 
 void G1CollectedHeap::register_old_region_with_region_attr(HeapRegion* r) {
   _region_attr.set_in_old(r->hrm_index(), r->rem_set()->is_tracked());
+  _rem_set->prepare_for_scan_rem_set(r->hrm_index());
 }
 
 void G1CollectedHeap::register_optional_region_with_region_attr(HeapRegion* r) {

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

@@ -73,9 +73,10 @@ public:
 
 // Used during Optional RS scanning to make sure we trim the queues in a timely manner.
 class G1ScanRSForOptionalClosure : public OopClosure {
+  G1CollectedHeap* _g1h;
   G1ScanCardClosure* _scan_cl;
 public:
-  G1ScanRSForOptionalClosure(G1ScanCardClosure* cl) : _scan_cl(cl) { }
+  G1ScanRSForOptionalClosure(G1CollectedHeap* g1h, G1ScanCardClosure* cl) : _g1h(g1h), _scan_cl(cl) { }
 
   template <class T> void do_oop_work(T* p);
   virtual void do_oop(oop* p)          { do_oop_work(p); }

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

@@ -169,11 +169,14 @@ inline void G1ScanCardClosure::do_oop_work(T* p) {
 
   check_obj_during_refinement(p, obj);
 
-  // We can not check for references from the collection set: the remembered sets
-  // may contain such entries and we do not filter them before.
+  assert(!_g1h->is_in_cset((HeapWord*)p),
+         "Oop originates from " PTR_FORMAT " (region: %u) which is in the collection set.",
+         p2i(p), _g1h->addr_to_region((HeapWord*)p));
 
   const G1HeapRegionAttr region_attr = _g1h->region_attr(obj);
   if (region_attr.is_in_cset()) {
+    // Since the source is always from outside the collection set, here we implicitly know
+    // that this is a cross-region reference too.
     prefetch_and_push(p, obj);
   } else if (!HeapRegion::is_in_same_region(p, obj)) {
     handle_non_cset_obj_common(region_attr, p, obj);
@@ -183,6 +186,13 @@ inline void G1ScanCardClosure::do_oop_work(T* p) {
 
 template <class T>
 inline void G1ScanRSForOptionalClosure::do_oop_work(T* p) {
+  const G1HeapRegionAttr region_attr = _g1h->region_attr(p);
+  // Entries in the optional collection set may start to originate from the collection
+  // set after one or more increments. In this case, previously optional regions
+  // became actual collection set regions. Filter them out here.
+  if (region_attr.is_in_cset()) {
+    return;
+  }
   _scan_cl->do_oop_work(p);
   _scan_cl->trim_queue_partially();
 }

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

@@ -208,6 +208,8 @@ template <typename T>
 inline void G1ParScanThreadState::remember_root_into_optional_region(T* p) {
   oop o = RawAccess<IS_NOT_NULL>::oop_load(p);
   uint index = _g1h->heap_region_containing(o)->index_in_opt_cset();
+  assert(index < _num_optional_regions,
+         "Trying to access optional region idx %u beyond " SIZE_FORMAT, index, _num_optional_regions);
   _oops_into_optional_regions[index].push_root(p);
 }
 
@@ -215,11 +217,16 @@ template <typename T>
 inline void G1ParScanThreadState::remember_reference_into_optional_region(T* p) {
   oop o = RawAccess<IS_NOT_NULL>::oop_load(p);
   uint index = _g1h->heap_region_containing(o)->index_in_opt_cset();
+  assert(index < _num_optional_regions,
+         "Trying to access optional region idx %u beyond " SIZE_FORMAT, index, _num_optional_regions);
   _oops_into_optional_regions[index].push_oop(p);
   DEBUG_ONLY(verify_ref(p);)
 }
 
 G1OopStarChunkedList* G1ParScanThreadState::oops_into_optional_region(const HeapRegion* hr) {
+  assert(hr->index_in_opt_cset() < _num_optional_regions,
+         "Trying to access optional region idx %u beyond " SIZE_FORMAT " " HR_FORMAT,
+         hr->index_in_opt_cset(), _num_optional_regions, HR_FORMAT_PARAMS(hr));
   return &_oops_into_optional_regions[hr->index_in_opt_cset()];
 }
 

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

@@ -188,7 +188,7 @@ public:
   void reset() {
     for (uint i = 0; i < _max_regions; i++) {
       _iter_states[i] = Unclaimed;
-      _scan_top[i] = NULL;
+      clear_scan_top(i);
     }
 
     G1ResetScanTopClosure cl(_scan_top);
@@ -253,6 +253,10 @@ public:
     return _scan_top[region_idx];
   }
 
+  void clear_scan_top(uint region_idx) {
+    _scan_top[region_idx] = NULL;
+  }
+
   // Clear the card table of "dirty" regions.
   void clear_card_table(WorkGang* workers) {
     if (_cur_dirty_region == 0) {
@@ -346,7 +350,7 @@ void G1ScanRSForRegionClosure::scan_opt_rem_set_roots(HeapRegion* r) {
   G1OopStarChunkedList* opt_rem_set_list = _pss->oops_into_optional_region(r);
 
   G1ScanCardClosure scan_cl(_g1h, _pss);
-  G1ScanRSForOptionalClosure cl(&scan_cl);
+  G1ScanRSForOptionalClosure cl(_g1h, &scan_cl);
   _opt_refs_scanned += opt_rem_set_list->oops_do(&cl, _pss->closures()->raw_strong_oops());
   _opt_refs_memory_used += opt_rem_set_list->used_memory();
 
@@ -550,6 +554,10 @@ void G1RemSet::prepare_for_scan_rem_set() {
   _scan_state->reset();
 }
 
+void G1RemSet::prepare_for_scan_rem_set(uint region_idx) {
+  _scan_state->clear_scan_top(region_idx);
+}
+
 void G1RemSet::cleanup_after_scan_rem_set() {
   G1GCPhaseTimes* phase_times = _g1h->phase_times();
 

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

@@ -98,10 +98,12 @@ public:
   // into the collection set or update the remembered set.
   void update_rem_set(G1ParScanThreadState* pss, uint worker_i);
 
-  // Prepare for and cleanup after scanning the remembered sets.  Must be called
+  // Prepare for and cleanup after scanning the remembered sets. Must be called
   // once before and after in sequential code.
   void prepare_for_scan_rem_set();
   void cleanup_after_scan_rem_set();
+  // Prepares the given region for remembered set scanning.
+  void prepare_for_scan_rem_set(uint region_idx);
 
   G1RemSetScanState* scan_state() const { return _scan_state; }