8212911: Unify and micro-optimize handling of non-in-collection set references in oop closures

Reviewed-by: kbarrett, sjohanss

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

@@ -38,7 +38,7 @@ G1ParCopyHelper::G1ParCopyHelper(G1CollectedHeap* g1h,  G1ParScanThreadState* pa
 { }
 
 G1ScanClosureBase::G1ScanClosureBase(G1CollectedHeap* g1h, G1ParScanThreadState* par_scan_state) :
-  _g1h(g1h), _par_scan_state(par_scan_state), _from(NULL)
+  _g1h(g1h), _par_scan_state(par_scan_state)
 { }
 
 void G1CLDScanClosure::do_cld(ClassLoaderData* cld) {

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

@@ -43,7 +43,6 @@ class G1ScanClosureBase : public BasicOopIterateClosure {
 protected:
   G1CollectedHeap* _g1h;
   G1ParScanThreadState* _par_scan_state;
-  HeapRegion* _from;
 
   G1ScanClosureBase(G1CollectedHeap* g1h, G1ParScanThreadState* par_scan_state);
   ~G1ScanClosureBase() { }
@@ -56,24 +55,19 @@ protected:
 public:
   virtual ReferenceIterationMode reference_iteration_mode() { return DO_FIELDS; }
 
-  void set_region(HeapRegion* from) { _from = from; }
-
   inline void trim_queue_partially();
 };
 
 // Used during the Update RS phase to refine remaining cards in the DCQ during garbage collection.
-class G1ScanObjsDuringUpdateRSClosure: public G1ScanClosureBase {
-  uint _worker_i;
-
+class G1ScanObjsDuringUpdateRSClosure : public G1ScanClosureBase {
 public:
   G1ScanObjsDuringUpdateRSClosure(G1CollectedHeap* g1h,
-                                  G1ParScanThreadState* pss,
-                                  uint worker_i) :
-    G1ScanClosureBase(g1h, pss), _worker_i(worker_i) { }
+                                  G1ParScanThreadState* pss) :
+    G1ScanClosureBase(g1h, pss) { }
 
   template <class T> void do_oop_work(T* p);
   virtual void do_oop(narrowOop* p) { do_oop_work(p); }
-  virtual void do_oop(oop* p) { do_oop_work(p); }
+  virtual void do_oop(oop* p)       { do_oop_work(p); }
 };
 
 // Used during the Scan RS phase to scan cards from the remembered set during garbage collection.
@@ -90,9 +84,13 @@ public:
 
 // This closure is applied to the fields of the objects that have just been copied during evacuation.
 class G1ScanEvacuatedObjClosure : public G1ScanClosureBase {
+  bool _scanning_in_young;
+
 public:
   G1ScanEvacuatedObjClosure(G1CollectedHeap* g1h, G1ParScanThreadState* par_scan_state) :
-    G1ScanClosureBase(g1h, par_scan_state) { }
+    G1ScanClosureBase(g1h, par_scan_state), _scanning_in_young(false) { }
+
+  void set_scanning_in_young(bool scanning_in_young) { _scanning_in_young = scanning_in_young; }
 
   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

@@ -82,12 +82,12 @@ inline void G1ScanEvacuatedObjClosure::do_oop_work(T* p) {
   const InCSetState state = _g1h->in_cset_state(obj);
   if (state.is_in_cset()) {
     prefetch_and_push(p, obj);
-  } else {
-    if (HeapRegion::is_in_same_region(p, obj)) {
+  } else if (!HeapRegion::is_in_same_region(p, obj)) {
+    handle_non_cset_obj_common(state, p, obj);
+    if (_scanning_in_young) {
       return;
     }
-    handle_non_cset_obj_common(state, p, obj);
-    _par_scan_state->update_rs(_from, p, obj);
+    _par_scan_state->enqueue_card_if_tracked(p, obj);
   }
 }
 
@@ -172,13 +172,9 @@ inline void G1ScanObjsDuringUpdateRSClosure::do_oop_work(T* p) {
     // 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 {
-    HeapRegion* to = _g1h->heap_region_containing(obj);
-    if (_from == to) {
-      return;
-    }
+  } else if (!HeapRegion::is_in_same_region(p, obj)) {
     handle_non_cset_obj_common(state, p, obj);
-    to->rem_set()->add_reference(p, _worker_i);
+    _par_scan_state->enqueue_card_if_tracked(p, obj);
   }
 }
 
@@ -193,10 +189,7 @@ inline void G1ScanObjsDuringScanRSClosure::do_oop_work(T* p) {
   const InCSetState state = _g1h->in_cset_state(obj);
   if (state.is_in_cset()) {
     prefetch_and_push(p, obj);
-  } else {
-    if (HeapRegion::is_in_same_region(p, obj)) {
-      return;
-    }
+  } else if (!HeapRegion::is_in_same_region(p, obj)) {
     handle_non_cset_obj_common(state, p, obj);
   }
 }

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

@@ -311,8 +311,7 @@ oop G1ParScanThreadState::copy_to_survivor_space(InCSetState const state,
       oop* old_p = set_partial_array_mask(old);
       do_oop_partial_array(old_p);
     } else {
-      HeapRegion* const to_region = _g1h->heap_region_containing(obj_ptr);
-      _scanner.set_region(to_region);
+      _scanner.set_scanning_in_young(dest_state.is_young());
       obj->oop_iterate_backwards(&_scanner);
     }
     return obj;
@@ -367,7 +366,7 @@ oop G1ParScanThreadState::handle_evacuation_failure_par(oop old, markOop m) {
 
     _g1h->preserve_mark_during_evac_failure(_worker_id, old, m);
 
-    _scanner.set_region(r);
+    _scanner.set_scanning_in_young(r->is_young());
     old->oop_iterate_backwards(&_scanner);
 
     return old;

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

@@ -104,17 +104,16 @@ public:
   template <class T> void do_oop_ext(T* ref);
   template <class T> void push_on_queue(T* ref);
 
-  template <class T> void update_rs(HeapRegion* from, T* p, oop o) {
-    assert(!HeapRegion::is_in_same_region(p, o), "Caller should have filtered out cross-region references already.");
-    // If the field originates from the to-space, we don't need to include it
-    // in the remembered set updates. Also, if we are not tracking the remembered
-    // set in the destination region, do not bother either.
-    if (!from->is_young() && _g1h->heap_region_containing((HeapWord*)o)->rem_set()->is_tracked()) {
-      size_t card_index = ct()->index_for(p);
-      // If the card hasn't been added to the buffer, do it.
-      if (ct()->mark_card_deferred(card_index)) {
-        dirty_card_queue().enqueue((jbyte*)ct()->byte_for_index(card_index));
-      }
+  template <class T> void enqueue_card_if_tracked(T* p, oop o) {
+    assert(!HeapRegion::is_in_same_region(p, o), "Should have filtered out cross-region references already.");
+    assert(!_g1h->heap_region_containing(p)->is_young(), "Should have filtered out from-young references already.");
+    if (!_g1h->heap_region_containing((HeapWord*)o)->rem_set()->is_tracked()) {
+      return;
+    }
+    size_t card_index = ct()->index_for(p);
+    // If the card hasn't been added to the buffer, do it.
+    if (ct()->mark_card_deferred(card_index)) {
+      dirty_card_queue().enqueue((jbyte*)ct()->byte_for_index(card_index));
     }
   }
 

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

@@ -61,9 +61,12 @@ template <class T> void G1ParScanThreadState::do_oop_evac(T* p) {
   RawAccess<IS_NOT_NULL>::oop_store(p, obj);
 
   assert(obj != NULL, "Must be");
-  if (!HeapRegion::is_in_same_region(p, obj)) {
-    HeapRegion* from = _g1h->heap_region_containing(p);
-    update_rs(from, p, obj);
+  if (HeapRegion::is_in_same_region(p, obj)) {
+    return;
+  }
+  HeapRegion* from = _g1h->heap_region_containing(p);
+  if (!from->is_young()) {
+    enqueue_card_if_tracked(p, obj);
   }
 }
 
@@ -109,7 +112,9 @@ inline void G1ParScanThreadState::do_oop_partial_array(oop* p) {
     // so that the heap remains parsable in case of evacuation failure.
     to_obj_array->set_length(end);
   }
-  _scanner.set_region(_g1h->heap_region_containing(to_obj));
+
+  HeapRegion* hr = _g1h->heap_region_containing(to_obj);
+  _scanner.set_scanning_in_young(hr->is_young());
   // Process indexes [start,end). It will also process the header
   // along with the first chunk (i.e., the chunk with start == 0).
   // Note that at this point the length field of to_obj_array is not

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

@@ -334,7 +334,7 @@ void G1ScanRSForRegionClosure::claim_card(size_t card_index, const uint region_i
 
 void G1ScanRSForRegionClosure::scan_card(MemRegion mr, uint region_idx_for_card) {
   HeapRegion* const card_region = _g1h->region_at(region_idx_for_card);
-  _scan_objs_on_card_cl->set_region(card_region);
+  assert(!card_region->is_young(), "Should not scan card in young region %u", region_idx_for_card);
   card_region->oops_on_card_seq_iterate_careful<true>(mr, _scan_objs_on_card_cl);
   _scan_objs_on_card_cl->trim_queue_partially();
   _cards_scanned++;
@@ -494,7 +494,7 @@ void G1RemSet::update_rem_set(G1ParScanThreadState* pss, uint worker_i) {
   if (G1HotCardCache::default_use_cache()) {
     G1EvacPhaseTimesTracker x(p, pss, G1GCPhaseTimes::ScanHCC, worker_i);
 
-    G1ScanObjsDuringUpdateRSClosure scan_hcc_cl(_g1h, pss, worker_i);
+    G1ScanObjsDuringUpdateRSClosure scan_hcc_cl(_g1h, pss);
     G1RefineCardClosure refine_card_cl(_g1h, &scan_hcc_cl);
     _g1h->iterate_hcc_closure(&refine_card_cl, worker_i);
   }
@@ -503,7 +503,7 @@ void G1RemSet::update_rem_set(G1ParScanThreadState* pss, uint worker_i) {
   {
     G1EvacPhaseTimesTracker x(p, pss, G1GCPhaseTimes::UpdateRS, worker_i);
 
-    G1ScanObjsDuringUpdateRSClosure update_rs_cl(_g1h, pss, worker_i);
+    G1ScanObjsDuringUpdateRSClosure update_rs_cl(_g1h, pss);
     G1RefineCardClosure refine_card_cl(_g1h, &update_rs_cl);
     _g1h->iterate_dirty_card_closure(&refine_card_cl, worker_i);
 
@@ -729,7 +729,7 @@ bool G1RemSet::refine_card_during_gc(jbyte* card_ptr,
   assert(!dirty_region.is_empty(), "sanity");
 
   HeapRegion* const card_region = _g1h->region_at(card_region_idx);
-  update_rs_cl->set_region(card_region);
+  assert(!card_region->is_young(), "Should not scan card in young region %u", card_region_idx);
   bool card_processed = card_region->oops_on_card_seq_iterate_careful<true>(dirty_region, update_rs_cl);
   assert(card_processed, "must be");
   return true;