RFR: 8270842: G1: Only young regions need to redirty outside references in remset.

[Hamlin Li]

On Wed, 21 Jul 2021 02:13:38 GMT, Hamlin Li <mli at openjdk.org> wrote:

> For evac failure objects in non-young regions (old) of cset, the outside reference remset already recorded in the dirty queue, we only needs to do it for obj in young regions

Sorry for delayed reply, I've been occupied by other things.
I'm afraid the solution (hacking G1ScanInYoungSetter) does not work as expected. With following super simple patch, I got a crash when Concurrent Mark starts (after evacuation failure in young regions, I triggerred it with G1EvacuationFailureALot).
'''
diff --git a/src/hotspot/share/gc/g1/g1EvacFailure.cpp b/src/hotspot/share/gc/g1/g1EvacFailure.cpp
index 1f775badbac..1ac1a69fc0a 100644
--- a/src/hotspot/share/gc/g1/g1EvacFailure.cpp
+++ b/src/hotspot/share/gc/g1/g1EvacFailure.cpp
@@ -155,7 +155,7 @@ public:
       // remembered set entries missing given that we skipped cards on
       // the collection set. So, we'll recreate such entries now.
       if (_is_young) {
-        obj->oop_iterate(_log_buffer_cl);
+        //obj->oop_iterate(_log_buffer_cl);
       }

       HeapWord* obj_end = obj_addr + obj_size;
diff --git a/src/hotspot/share/gc/g1/g1ParScanThreadState.cpp b/src/hotspot/share/gc/g1/g1ParScanThreadState.cpp
index 9eebe411936..7e82f50b373 100644
--- a/src/hotspot/share/gc/g1/g1ParScanThreadState.cpp
+++ b/src/hotspot/share/gc/g1/g1ParScanThreadState.cpp
@@ -611,7 +611,7 @@ oop G1ParScanThreadState::handle_evacuation_failure_par(oop old, markWord m) {

     _g1h->preserve_mark_during_evac_failure(_worker_id, old, m);

-    G1ScanInYoungSetter x(&_scanner, r->is_young());
+    G1ScanInYoungSetter x(&_scanner, false);
     old->oop_iterate_backwards(&_scanner);

     return old;
diff --git a/src/hotspot/share/gc/g1/g1ParScanThreadState.hpp b/src/hotspot/share/gc/g1/g1ParScanThreadState.hpp
index 79f5b3a22cb..2a2060642cc 100644
--- a/src/hotspot/share/gc/g1/g1ParScanThreadState.hpp
+++ b/src/hotspot/share/gc/g1/g1ParScanThreadState.hpp
@@ -130,7 +130,7 @@ public:

   template <class T> void enqueue_card_if_tracked(G1HeapRegionAttr region_attr, 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.");
+    // assert(!_g1h->heap_region_containing(p)->is_young(), "Should have filtered out from-young references already.");

 #ifdef ASSERT
     HeapRegion* const hr_obj = _g1h->heap_region_containing(o);
'''

After investigation, I think it's because the difference between the initial patch (says pathc I) and the patch hacking G1ScanInYoungSetter (says patch H) is that:
1. Patch I keep all the logic in UpdateLogBuffersDeferred for evac failure obj in young regions, especially it will enqueue all the fields for evac failure obj (check UpdateLogBuffersDeferred::do_oop_work(T* p), not matter where it points to(except of pointing to its own region).
2. Patch H will not do it for fields of evac failure obj in young regions if a field points to obj in cset. (check G1ScanEvacuatedObjClosure::do_oop_work(T* p), G1ParScanThreadState::do_oop_evac(T* p))

If I amend following patch (says Patch A), it will resolve the crash issue, but I don't think this is what we want.
'''
diff --git a/src/hotspot/share/gc/g1/g1ParScanThreadState.cpp b/src/hotspot/share/gc/g1/g1ParScanThreadState.cpp
index 9eebe411936..91d9bc76528 100644
--- a/src/hotspot/share/gc/g1/g1ParScanThreadState.cpp
+++ b/src/hotspot/share/gc/g1/g1ParScanThreadState.cpp
@@ -208,9 +208,9 @@ void G1ParScanThreadState::do_oop_evac(T* p) {
     return;
   }
   HeapRegion* from = _g1h->heap_region_containing(p);
-  if (!from->is_young()) {
+
     enqueue_card_if_tracked(_g1h->region_attr(obj), p, obj);
-  }
+
'''

I still don't know the exact reason why the Patch H will fail, not sure if you will have some clue?
But seems the Patch I is the right way to go.

-------------

PR: https://git.openjdk.java.net/jdk/pull/4853