8370041: GenShen: Filter young pointers from thread local SATB buffers when only marking old

Reviewed-by: ysr, kdnilsen

src/hotspot/share/gc/shenandoah/shenandoahClosures.hpp

@@ -24,7 +24,6 @@
 #ifndef SHARE_GC_SHENANDOAH_SHENANDOAHCLOSURES_HPP
 #define SHARE_GC_SHENANDOAH_SHENANDOAHCLOSURES_HPP
 
-#include "code/nmethod.hpp"
 #include "gc/shared/stringdedup/stringDedup.hpp"
 #include "gc/shenandoah/shenandoahGenerationType.hpp"
 #include "gc/shenandoah/shenandoahTaskqueue.hpp"
@@ -230,6 +229,17 @@ public:
 };
 
 
+class ShenandoahFlushSATB : public ThreadClosure {
+private:
+  SATBMarkQueueSet& _satb_qset;
+
+public:
+  explicit ShenandoahFlushSATB(SATBMarkQueueSet& satb_qset) : _satb_qset(satb_qset) {}
+
+  inline void do_thread(Thread* thread) override;
+};
+
+
 //
 // ========= Utilities
 //

src/hotspot/share/gc/shenandoah/shenandoahClosures.inline.hpp

@@ -253,6 +253,10 @@ inline void ShenandoahConcUpdateRefsClosure::work(T* p) {
   _heap->conc_update_with_forwarded(p);
 }
 
+inline void ShenandoahFlushSATB::do_thread(Thread* thread) {
+  // Transfer any partial buffer to the qset for completed buffer processing.
+  _satb_qset.flush_queue(ShenandoahThreadLocalData::satb_mark_queue(thread));
+}
 
 //
 // ========= Utilities

src/hotspot/share/gc/shenandoah/shenandoahConcurrentGC.cpp

@@ -279,21 +279,6 @@ bool ShenandoahConcurrentGC::complete_abbreviated_cycle() {
     heap->update_region_ages(_generation->complete_marking_context());
   }
 
-  if (!heap->is_concurrent_old_mark_in_progress()) {
-    heap->concurrent_final_roots();
-  } else {
-    // Since the cycle was shortened for having enough immediate garbage, this will be
-    // the last phase before concurrent marking of old resumes. We must be sure
-    // that old mark threads don't see any pointers to garbage in the SATB queues. Even
-    // though nothing was evacuated, overwriting unreachable weak roots with null may still
-    // put pointers to regions that become trash in the SATB queues. The following will
-    // piggyback flushing the thread local SATB queues on the same handshake that propagates
-    // the gc state change.
-    ShenandoahSATBMarkQueueSet& satb_queues = ShenandoahBarrierSet::satb_mark_queue_set();
-    ShenandoahFlushSATBHandshakeClosure complete_thread_local_satb_buffers(satb_queues);
-    heap->concurrent_final_roots(&complete_thread_local_satb_buffers);
-    heap->old_generation()->concurrent_transfer_pointers_from_satb();
-  }
   return true;
 }
 
@@ -684,16 +669,10 @@ void ShenandoahConcurrentGC::op_init_mark() {
   assert(!heap->has_forwarded_objects(), "No forwarded objects on this path");
 
   if (heap->mode()->is_generational()) {
-
     if (_generation->is_global()) {
       heap->old_generation()->cancel_gc();
-    } else if (heap->is_concurrent_old_mark_in_progress()) {
-      // Purge the SATB buffers, transferring any valid, old pointers to the
-      // old generation mark queue. Any pointers in a young region will be
-      // abandoned.
-      ShenandoahGCPhase phase(ShenandoahPhaseTimings::init_transfer_satb);
-      heap->old_generation()->transfer_pointers_from_satb();
     }
+
     {
       // After we swap card table below, the write-table is all clean, and the read table holds
       // cards dirty prior to the start of GC. Young and bootstrap collection will update
@@ -1131,7 +1110,7 @@ private:
   ShenandoahNonConcUpdateRefsClosure _cl;
 public:
   ShenandoahUpdateThreadHandshakeClosure();
-  void do_thread(Thread* thread);
+  void do_thread(Thread* thread) override;
 };
 
 ShenandoahUpdateThreadHandshakeClosure::ShenandoahUpdateThreadHandshakeClosure() :
@@ -1146,9 +1125,49 @@ void ShenandoahUpdateThreadHandshakeClosure::do_thread(Thread* thread) {
   }
 }
 
+class ShenandoahUpdateThreadRootsAndFlushOldSatbBuffers final : public HandshakeClosure {
+  // When Shenandoah is marking the old generation, it is possible for the SATB barrier
+  // to pick up overwritten pointers that point into a cset region. If these pointers
+  // are accessed by mark threads, they will crash. Once update refs has completed, it is
+  // no longer possible for a mutator thread to overwrite a pointer into a cset region.
+  //
+  // Therefore, at the end of update refs, we use this closure to update the thread roots
+  // and 'complete' all the thread local SATB buffers. Completing these will filter out
+  // anything that has already been marked or anything that points to a region which is
+  // not old. We do not need to worry about ABA situations where a region may become old
+  // after the pointer is enqueued but before it is filtered. There are only two ways a
+  // region may become old:
+  //  1. The region is promoted in place. This is safe because such regions will never
+  //     be in the collection set. If this happens, the pointer will be preserved, essentially
+  //     becoming part of the old snapshot.
+  //  2. The region is allocated during evacuation of old. This is also not a concern because
+  //     we haven't yet finished marking old so no mixed evacuations will happen.
+  ShenandoahUpdateThreadHandshakeClosure _update_roots;
+  ShenandoahFlushSATB _flush_all_satb;
+
+public:
+  ShenandoahUpdateThreadRootsAndFlushOldSatbBuffers() :
+    HandshakeClosure("Shenandoah Update Thread Roots and Flush SATB"),
+    _flush_all_satb(ShenandoahBarrierSet::satb_mark_queue_set()) {
+    assert(ShenandoahBarrierSet::satb_mark_queue_set().get_filter_out_young(),
+           "Should be filtering pointers outside of old during old marking");
+  }
+
+  void do_thread(Thread* thread) override {
+    _update_roots.do_thread(thread);
+    _flush_all_satb.do_thread(thread);
+  }
+};
+
 void ShenandoahConcurrentGC::op_update_thread_roots() {
-  ShenandoahUpdateThreadHandshakeClosure cl;
-  Handshake::execute(&cl);
+  ShenandoahHeap* const heap = ShenandoahHeap::heap();
+  if (heap->is_concurrent_old_mark_in_progress()) {
+    ShenandoahUpdateThreadRootsAndFlushOldSatbBuffers cl;
+    Handshake::execute(&cl);
+  } else {
+    ShenandoahUpdateThreadHandshakeClosure cl;
+    Handshake::execute(&cl);
+  }
 }
 
 void ShenandoahConcurrentGC::op_final_update_refs() {
@@ -1177,23 +1196,6 @@ void ShenandoahConcurrentGC::op_final_update_refs() {
   heap->set_has_forwarded_objects(false);
 
   if (heap->mode()->is_generational() && heap->is_concurrent_old_mark_in_progress()) {
-    // When the SATB barrier is left on to support concurrent old gen mark, it may pick up writes to
-    // objects in the collection set. After those objects are evacuated, the pointers in the
-    // SATB are no longer safe. Once we have finished update references, we are guaranteed that
-    // no more writes to the collection set are possible.
-    //
-    // This will transfer any old pointers in _active_ regions from the SATB to the old gen
-    // mark queues. All other pointers will be discarded. This would also discard any pointers
-    // in old regions that were included in a mixed evacuation. We aren't using the SATB filter
-    // methods here because we cannot control when they execute. If the SATB filter runs _after_
-    // a region has been recycled, we will not be able to detect the bad pointer.
-    //
-    // We are not concerned about skipping this step in abbreviated cycles because regions
-    // with no live objects cannot have been written to and so cannot have entries in the SATB
-    // buffers.
-    ShenandoahGCPhase phase(ShenandoahPhaseTimings::final_update_refs_transfer_satb);
-    heap->old_generation()->transfer_pointers_from_satb();
-
     // Aging_cycle is only relevant during evacuation cycle for individual objects and during final mark for
     // entire regions.  Both of these relevant operations occur before final update refs.
     ShenandoahGenerationalHeap::heap()->set_aging_cycle(false);
@@ -1228,13 +1230,13 @@ bool ShenandoahConcurrentGC::entry_final_roots() {
                               ShenandoahWorkerPolicy::calc_workers_for_conc_evac(),
                               msg);
 
-  if (!heap->mode()->is_generational()) {
-    heap->concurrent_final_roots();
-  } else {
+  if (heap->mode()->is_generational()) {
     if (!complete_abbreviated_cycle()) {
       return false;
     }
   }
+
+  heap->concurrent_final_roots();
   return true;
 }
 

src/hotspot/share/gc/shenandoah/shenandoahConcurrentMark.cpp

@@ -66,20 +66,6 @@ public:
   }
 };
 
-class ShenandoahSATBAndRemarkThreadsClosure : public ThreadClosure {
-private:
-  SATBMarkQueueSet& _satb_qset;
-
-public:
-  explicit ShenandoahSATBAndRemarkThreadsClosure(SATBMarkQueueSet& satb_qset) :
-    _satb_qset(satb_qset) {}
-
-  void do_thread(Thread* thread) override {
-    // Transfer any partial buffer to the qset for completed buffer processing.
-    _satb_qset.flush_queue(ShenandoahThreadLocalData::satb_mark_queue(thread));
-  }
-};
-
 template <ShenandoahGenerationType GENERATION>
 class ShenandoahFinalMarkingTask : public WorkerTask {
 private:
@@ -109,7 +95,7 @@ public:
       while (satb_mq_set.apply_closure_to_completed_buffer(&cl)) {}
       assert(!heap->has_forwarded_objects(), "Not expected");
 
-      ShenandoahSATBAndRemarkThreadsClosure tc(satb_mq_set);
+      ShenandoahFlushSATB tc(satb_mq_set);
       Threads::possibly_parallel_threads_do(true /* is_par */, &tc);
     }
     _cm->mark_loop(worker_id, _terminator, GENERATION, false /*not cancellable*/,

src/hotspot/share/gc/shenandoah/shenandoahDegeneratedGC.cpp

@@ -101,12 +101,16 @@ void ShenandoahDegenGC::op_degenerated() {
     heap->old_generation()->card_scan()->mark_write_table_as_clean();
   }
 
-#ifdef ASSERT
   if (heap->mode()->is_generational()) {
-    ShenandoahOldGeneration* old_generation = heap->old_generation();
+    const ShenandoahOldGeneration* old_generation = heap->old_generation();
     if (!heap->is_concurrent_old_mark_in_progress()) {
       // If we are not marking the old generation, there should be nothing in the old mark queues
       assert(old_generation->task_queues()->is_empty(), "Old gen task queues should be empty");
+    } else {
+      // This is still necessary for degenerated cycles because the degeneration point may occur
+      // after final mark of the young generation. See ShenandoahConcurrentGC::op_final_update_refs for
+      // a more detailed explanation.
+      old_generation->transfer_pointers_from_satb();
     }
 
     if (_generation->is_global()) {
@@ -118,7 +122,6 @@ void ShenandoahDegenGC::op_degenerated() {
              "Old generation cannot be in state: %s", old_generation->state_name());
     }
   }
-#endif
 
   ShenandoahMetricsSnapshot metrics(heap->free_set());
 
@@ -166,15 +169,6 @@ void ShenandoahDegenGC::op_degenerated() {
           _generation->cancel_marking();
         }
 
-        if (heap->is_concurrent_mark_in_progress()) {
-          // If either old or young marking is in progress, the SATB barrier will be enabled.
-          // The SATB buffer may hold a mix of old and young pointers. The old pointers need to be
-          // transferred to the old generation mark queues and the young pointers are NOT part
-          // of this snapshot, so they must be dropped here. It is safe to drop them here because
-          // we will rescan the roots on this safepoint.
-          heap->old_generation()->transfer_pointers_from_satb();
-        }
-
         if (_degen_point == ShenandoahDegenPoint::_degenerated_roots) {
           // We only need this if the concurrent cycle has already swapped the card tables.
           // Marking will use the 'read' table, but interesting pointers may have been
@@ -193,8 +187,9 @@ void ShenandoahDegenGC::op_degenerated() {
     case _degenerated_mark:
       // No fallthrough. Continue mark, handed over from concurrent mark if
       // concurrent mark has yet completed
-      if (_degen_point == ShenandoahDegenPoint::_degenerated_mark &&
-          heap->is_concurrent_mark_in_progress()) {
+      if (_degen_point == ShenandoahDegenPoint::_degenerated_mark && heap->is_concurrent_mark_in_progress()) {
+        assert(!ShenandoahBarrierSet::satb_mark_queue_set().get_filter_out_young(),
+               "Should not be filtering out young pointers when concurrent mark degenerates");
         op_finish_mark();
       }
       assert(!heap->cancelled_gc(), "STW mark can not OOM");

src/hotspot/share/gc/shenandoah/shenandoahGenerationalHeap.cpp

@@ -1040,12 +1040,6 @@ void ShenandoahGenerationalHeap::final_update_refs_update_region_states() {
 
 void ShenandoahGenerationalHeap::complete_degenerated_cycle() {
   shenandoah_assert_heaplocked_or_safepoint();
-  if (is_concurrent_old_mark_in_progress()) {
-    // This is still necessary for degenerated cycles because the degeneration point may occur
-    // after final mark of the young generation. See ShenandoahConcurrentGC::op_final_update_refs for
-    // a more detailed explanation.
-    old_generation()->transfer_pointers_from_satb();
-  }
   // In case degeneration interrupted concurrent evacuation or update references, we need to clean up
   // transient state. Otherwise, these actions have no effect.
   reset_generation_reserves();

src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp

@@ -2028,6 +2028,10 @@ void ShenandoahHeap::prepare_update_heap_references() {
 void ShenandoahHeap::propagate_gc_state_to_all_threads() {
   assert(ShenandoahSafepoint::is_at_shenandoah_safepoint(), "Must be at Shenandoah safepoint");
   if (_gc_state_changed) {
+    // If we are only marking old, we do not need to process young pointers
+    ShenandoahBarrierSet::satb_mark_queue_set().set_filter_out_young(
+      is_concurrent_old_mark_in_progress() && !is_concurrent_young_mark_in_progress()
+    );
     ShenandoahGCStatePropagatorHandshakeClosure propagator(_gc_state.raw_value());
     Threads::threads_do(&propagator);
     _gc_state_changed = false;

src/hotspot/share/gc/shenandoah/shenandoahOldGeneration.cpp

@@ -44,113 +44,17 @@
 #include "runtime/threads.hpp"
 #include "utilities/events.hpp"
 
-class ShenandoahFlushAllSATB : public ThreadClosure {
-private:
-  SATBMarkQueueSet& _satb_qset;
-
-public:
-  explicit ShenandoahFlushAllSATB(SATBMarkQueueSet& satb_qset) :
-    _satb_qset(satb_qset) {}
-
-  void do_thread(Thread* thread) override {
-    // Transfer any partial buffer to the qset for completed buffer processing.
-    _satb_qset.flush_queue(ShenandoahThreadLocalData::satb_mark_queue(thread));
-  }
-};
-
-class ShenandoahProcessOldSATB : public SATBBufferClosure {
-private:
-  ShenandoahObjToScanQueue*       _queue;
-  ShenandoahHeap*                 _heap;
-  ShenandoahMarkingContext* const _mark_context;
-  size_t                          _trashed_oops;
-
-public:
-  explicit ShenandoahProcessOldSATB(ShenandoahObjToScanQueue* q) :
-    _queue(q),
-    _heap(ShenandoahHeap::heap()),
-    _mark_context(_heap->marking_context()),
-    _trashed_oops(0) {}
-
-  void do_buffer(void** buffer, size_t size) override {
-    assert(size == 0 || !_heap->has_forwarded_objects() || _heap->is_concurrent_old_mark_in_progress(), "Forwarded objects are not expected here");
-    for (size_t i = 0; i < size; ++i) {
-      oop *p = (oop *) &buffer[i];
-      ShenandoahHeapRegion* region = _heap->heap_region_containing(*p);
-      if (region->is_old() && region->is_active()) {
-          ShenandoahMark::mark_through_ref<oop, OLD>(p, _queue, nullptr, _mark_context, false);
-      } else {
-        _trashed_oops++;
-      }
-    }
-  }
-
-  size_t trashed_oops() const {
-    return _trashed_oops;
-  }
-};
-
 class ShenandoahPurgeSATBTask : public WorkerTask {
-private:
-  ShenandoahObjToScanQueueSet* _mark_queues;
-  // Keep track of the number of oops that are not transferred to mark queues.
-  // This is volatile because workers update it, but the vm thread reads it.
-  volatile size_t             _trashed_oops;
-
 public:
-  explicit ShenandoahPurgeSATBTask(ShenandoahObjToScanQueueSet* queues) :
-    WorkerTask("Purge SATB"),
-    _mark_queues(queues),
-    _trashed_oops(0) {
+  explicit ShenandoahPurgeSATBTask() : WorkerTask("Purge SATB") {
     Threads::change_thread_claim_token();
   }
 
-  ~ShenandoahPurgeSATBTask() {
-    if (_trashed_oops > 0) {
-      log_debug(gc)("Purged %zu oops from old generation SATB buffers", _trashed_oops);
-    }
-  }
-
   void work(uint worker_id) override {
     ShenandoahParallelWorkerSession worker_session(worker_id);
     ShenandoahSATBMarkQueueSet &satb_queues = ShenandoahBarrierSet::satb_mark_queue_set();
-    ShenandoahFlushAllSATB flusher(satb_queues);
+    ShenandoahFlushSATB flusher(satb_queues);
     Threads::possibly_parallel_threads_do(true /* is_par */, &flusher);
-
-    ShenandoahObjToScanQueue* mark_queue = _mark_queues->queue(worker_id);
-    ShenandoahProcessOldSATB processor(mark_queue);
-    while (satb_queues.apply_closure_to_completed_buffer(&processor)) {}
-
-    AtomicAccess::add(&_trashed_oops, processor.trashed_oops());
-  }
-};
-
-class ShenandoahTransferOldSATBTask : public WorkerTask {
-  ShenandoahSATBMarkQueueSet&  _satb_queues;
-  ShenandoahObjToScanQueueSet* _mark_queues;
-  // Keep track of the number of oops that are not transferred to mark queues.
-  // This is volatile because workers update it, but the control thread reads it.
-  volatile size_t              _trashed_oops;
-
-public:
-  explicit ShenandoahTransferOldSATBTask(ShenandoahSATBMarkQueueSet& satb_queues, ShenandoahObjToScanQueueSet* mark_queues) :
-    WorkerTask("Transfer SATB"),
-    _satb_queues(satb_queues),
-    _mark_queues(mark_queues),
-    _trashed_oops(0) {}
-
-  ~ShenandoahTransferOldSATBTask() {
-    if (_trashed_oops > 0) {
-      log_debug(gc)("Purged %zu oops from old generation SATB buffers", _trashed_oops);
-    }
-  }
-
-  void work(uint worker_id) override {
-    ShenandoahObjToScanQueue* mark_queue = _mark_queues->queue(worker_id);
-    ShenandoahProcessOldSATB processor(mark_queue);
-    while (_satb_queues.apply_closure_to_completed_buffer(&processor)) {}
-
-    AtomicAccess::add(&_trashed_oops, processor.trashed_oops());
   }
 };
 
@@ -463,26 +367,11 @@ bool ShenandoahOldGeneration::coalesce_and_fill() {
   }
 }
 
-void ShenandoahOldGeneration::concurrent_transfer_pointers_from_satb() const {
-  const ShenandoahHeap* heap = ShenandoahHeap::heap();
-  assert(heap->is_concurrent_old_mark_in_progress(), "Only necessary during old marking.");
-  log_debug(gc)("Transfer SATB buffers");
-
-  // Step 1. All threads need to 'complete' partially filled, thread local SATB buffers. This
-  // is accomplished in ShenandoahConcurrentGC::complete_abbreviated_cycle using a Handshake
-  // operation.
-  // Step 2. Use worker threads to transfer oops from old, active regions in the completed
-  // SATB buffers to old generation mark queues.
-  ShenandoahSATBMarkQueueSet& satb_queues = ShenandoahBarrierSet::satb_mark_queue_set();
-  ShenandoahTransferOldSATBTask transfer_task(satb_queues, task_queues());
-  heap->workers()->run_task(&transfer_task);
-}
-
 void ShenandoahOldGeneration::transfer_pointers_from_satb() const {
   const ShenandoahHeap* heap = ShenandoahHeap::heap();
   assert(heap->is_concurrent_old_mark_in_progress(), "Only necessary during old marking.");
   log_debug(gc)("Transfer SATB buffers");
-  ShenandoahPurgeSATBTask purge_satb_task(task_queues());
+  ShenandoahPurgeSATBTask purge_satb_task;
   heap->workers()->run_task(&purge_satb_task);
 }
 

src/hotspot/share/gc/shenandoah/shenandoahOldGeneration.hpp

@@ -228,26 +228,27 @@ public:
   // Cancels old gc and transitions to the idle state
   void cancel_gc();
 
-  // We leave the SATB barrier on for the entirety of the old generation
-  // marking phase. In some cases, this can cause a write to a perfectly
-  // reachable oop to enqueue a pointer that later becomes garbage (because
-  // it points at an object that is later chosen for the collection set). There are
-  // also cases where the referent of a weak reference ends up in the SATB
-  // and is later collected. In these cases the oop in the SATB buffer becomes
-  // invalid and the _next_ cycle will crash during its marking phase. To
-  // avoid this problem, we "purge" the SATB buffers during the final update
-  // references phase if (and only if) an old generation mark is in progress.
-  // At this stage we can safely determine if any of the oops in the SATB
-  // buffer belong to trashed regions (before they are recycled). As it
-  // happens, flushing a SATB queue also filters out oops which have already
-  // been marked - which is the case for anything that is being evacuated
-  // from the collection set.
+  // The SATB barrier will be "enabled" until old marking completes. This means it is
+  // possible for an entire young collection cycle to execute while the SATB barrier is enabled.
+  // Consider a situation like this, where we have a pointer 'B' at an object 'A' which is in
+  // the young collection set:
   //
-  // Alternatively, we could inspect the state of the heap and the age of the
-  // object at the barrier, but we reject this approach because it is likely
-  // the performance impact would be too severe.
+  //      +--Young, CSet------+     +--Young, Regular----+
+  //      |                   |     |                    |
+  //      |                   |     |                    |
+  //      |       A <--------------------+ B             |
+  //      |                   |     |                    |
+  //      |                   |     |                    |
+  //      +-------------------+     +--------------------+
+  //
+  // If a mutator thread overwrites pointer B, the SATB barrier will dutifully enqueue
+  // object A. However, this object will be trashed when the young cycle completes. We must,
+  // therefore, filter this object from the SATB buffer before any old mark threads see it.
+  // We do this with a handshake before final-update-refs (see shenandoahConcurrentGC.cpp).
+  //
+  // This method is here only for degenerated cycles. A concurrent cycle may be cancelled before
+  // we have a chance to execute the handshake to flush the SATB in final-update-refs.
   void transfer_pointers_from_satb() const;
-  void concurrent_transfer_pointers_from_satb() const;
 
   // True if there are old regions waiting to be selected for a mixed collection
   bool has_unprocessed_collection_candidates();

src/hotspot/share/gc/shenandoah/shenandoahSATBMarkQueueSet.cpp

@@ -28,7 +28,7 @@
 #include "gc/shenandoah/shenandoahThreadLocalData.hpp"
 
 ShenandoahSATBMarkQueueSet::ShenandoahSATBMarkQueueSet(BufferNode::Allocator* allocator) :
-  SATBMarkQueueSet(allocator)
+  SATBMarkQueueSet(allocator), _filter_out_young(false)
 {}
 
 SATBMarkQueue& ShenandoahSATBMarkQueueSet::satb_queue_for_thread(Thread* const t) const {
@@ -39,16 +39,33 @@ class ShenandoahSATBMarkQueueFilterFn {
   ShenandoahHeap* const _heap;
 
 public:
-  ShenandoahSATBMarkQueueFilterFn(ShenandoahHeap* heap) : _heap(heap) {}
+  explicit ShenandoahSATBMarkQueueFilterFn(ShenandoahHeap* heap) : _heap(heap) {}
 
-  // Return true if entry should be filtered out (removed), false if
-  // it should be retained.
+  // Return true if entry should be filtered out (removed), false if it should be retained.
   bool operator()(const void* entry) const {
     return !_heap->requires_marking(entry);
   }
 };
 
+class ShenandoahSATBOldMarkQueueFilterFn {
+  ShenandoahHeap* const _heap;
+
+public:
+  explicit ShenandoahSATBOldMarkQueueFilterFn(ShenandoahHeap* heap) : _heap(heap) {}
+
+  // Return true if entry should be filtered out (removed), false if it should be retained.
+  bool operator()(const void* entry) const {
+    assert(_heap->is_concurrent_old_mark_in_progress(), "Should only use this when old marking is in progress");
+    assert(!_heap->is_concurrent_young_mark_in_progress(), "Should only use this when young marking is not in progress");
+    return !_heap->requires_marking(entry) || !_heap->is_in_old(entry);
+  }
+};
+
 void ShenandoahSATBMarkQueueSet::filter(SATBMarkQueue& queue) {
   ShenandoahHeap* heap = ShenandoahHeap::heap();
-  apply_filter(ShenandoahSATBMarkQueueFilterFn(heap), queue);
+  if (_filter_out_young) {
+    apply_filter(ShenandoahSATBOldMarkQueueFilterFn(heap), queue);
+  } else {
+    apply_filter(ShenandoahSATBMarkQueueFilterFn(heap), queue);
+  }
 }

src/hotspot/share/gc/shenandoah/shenandoahSATBMarkQueueSet.hpp

@@ -27,15 +27,23 @@
 
 #include "gc/shared/bufferNode.hpp"
 #include "gc/shared/satbMarkQueue.hpp"
-#include "runtime/javaThread.hpp"
-#include "runtime/mutex.hpp"
 
 class ShenandoahSATBMarkQueueSet : public SATBMarkQueueSet {
-public:
-  ShenandoahSATBMarkQueueSet(BufferNode::Allocator* allocator);
+private:
+  bool _filter_out_young;
 
-  virtual SATBMarkQueue& satb_queue_for_thread(Thread* const t) const;
-  virtual void filter(SATBMarkQueue& queue);
+public:
+  explicit ShenandoahSATBMarkQueueSet(BufferNode::Allocator* allocator);
+
+  SATBMarkQueue& satb_queue_for_thread(Thread* const t) const override;
+  void filter(SATBMarkQueue& queue) override;
+  void set_filter_out_young(bool filter_out_young) {
+    _filter_out_young = filter_out_young;
+  }
+
+  bool get_filter_out_young() const {
+    return _filter_out_young;
+  }
 };
 
 #endif // SHARE_GC_SHENANDOAH_SHENANDOAHSATBMARKQUEUESET_HPP