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

rev 50928 : Process remaining SATB buffers in final mark/traverse loop instead of separate phase

@@ -203,11 +203,10 @@
     }
 
     _cm->concurrent_scan_code_roots(worker_id, rp, _update_refs);
     _cm->mark_loop(worker_id, _terminator, rp,
                    true, // cancellable
-                   true, // drain SATBs as we go
                    _cm->unload_classes(),
                    _update_refs,
                    ShenandoahStringDedup::is_enabled()); // perform string dedup
   }
 };

@@ -231,11 +230,11 @@
     // First drain remaining SATB buffers.
     // Notice that this is not strictly necessary for mark-compact. But since
     // it requires a StrongRootsScope around the task, we need to claim the
     // threads, and performance-wise it doesn't really matter. Adds about 1ms to
     // full-gc.
-    _cm->drain_satb_buffers(worker_id, true);
+    _cm->drain_satb_buffers(worker_id);
 
     ReferenceProcessor* rp;
     if (_cm->process_references()) {
       rp = ShenandoahHeap::heap()->ref_processor();
       shenandoah_assert_rp_isalive_installed();

@@ -246,11 +245,10 @@
     // Degenerated cycle may bypass concurrent cycle, so code roots might not be scanned,
     // let's check here.
     _cm->concurrent_scan_code_roots(worker_id, rp, _update_refs);
     _cm->mark_loop(worker_id, _terminator, rp,
                    false, // not cancellable
-                   false, // do not drain SATBs, already drained
                    _unload_classes,
                    _update_refs,
                    _dedup_string);
 
     assert(_cm->task_queues()->is_empty(), "Should be empty");

@@ -497,21 +495,15 @@
       }
     }
   }
 };
 
-void ShenandoahConcurrentMark::drain_satb_buffers(uint worker_id, bool remark) {
+void ShenandoahConcurrentMark::drain_satb_buffers(uint worker_id) {
   ShenandoahObjToScanQueue* q = get_queue(worker_id);
   ShenandoahSATBBufferClosure cl(q);
-
-  SATBMarkQueueSet& satb_mq_set = ShenandoahBarrierSet::satb_mark_queue_set();
-  while (satb_mq_set.apply_closure_to_completed_buffer(&cl));
-
-  if (remark) {
     ShenandoahSATBThreadsClosure tc(&cl);
     Threads::threads_do(&tc);
-  }
 }
 
 #if TASKQUEUE_STATS
 void ShenandoahConcurrentMark::print_taskqueue_stats_hdr(outputStream* const st) {
   st->print_raw_cr("GC Task Stats");

@@ -573,11 +565,10 @@
 
     shenandoah_assert_rp_isalive_installed();
 
     scm->mark_loop(_worker_id, _terminator, rp,
                    false, // not cancellable
-                   false, // do not drain SATBs
                    scm->unload_classes(),
                    sh->has_forwarded_objects(),
                    false);  // do not do strdedup
 
     if (_reset_terminator) {

@@ -799,11 +790,10 @@
     ReferenceProcessor* rp = sh->ref_processor();
     shenandoah_assert_rp_isalive_installed();
 
     scm->mark_loop(0, &terminator, rp,
                    false, // not cancellable
-                   true,  // drain SATBs
                    scm->unload_classes(),
                    sh->has_forwarded_objects(),
                    false); // do not do strdedup
   }
 };

@@ -892,11 +882,11 @@
   q->set_empty();
   q->overflow_stack()->clear();
   q->clear_buffer();
 }
 
-template <bool CANCELLABLE, bool DRAIN_SATB>
+template <bool CANCELLABLE>
 void ShenandoahConcurrentMark::mark_loop_prework(uint w, ParallelTaskTerminator *t, ReferenceProcessor *rp,
                                                  bool class_unload, bool update_refs, bool strdedup) {
   ShenandoahObjToScanQueue* q = get_queue(w);
 
   jushort* ld = get_liveness(w);

@@ -907,43 +897,43 @@
   if (class_unload) {
     if (update_refs) {
       if (strdedup) {
         ShenandoahStrDedupQueue* dq = ShenandoahStringDedup::queue(w);
         ShenandoahMarkUpdateRefsMetadataDedupClosure cl(q, dq, rp);
-        mark_loop_work<ShenandoahMarkUpdateRefsMetadataDedupClosure, CANCELLABLE, DRAIN_SATB>(&cl, ld, w, t);
+        mark_loop_work<ShenandoahMarkUpdateRefsMetadataDedupClosure, CANCELLABLE>(&cl, ld, w, t);
       } else {
         ShenandoahMarkUpdateRefsMetadataClosure cl(q, rp);
-        mark_loop_work<ShenandoahMarkUpdateRefsMetadataClosure, CANCELLABLE, DRAIN_SATB>(&cl, ld, w, t);
+        mark_loop_work<ShenandoahMarkUpdateRefsMetadataClosure, CANCELLABLE>(&cl, ld, w, t);
       }
     } else {
       if (strdedup) {
         ShenandoahStrDedupQueue* dq = ShenandoahStringDedup::queue(w);
         ShenandoahMarkRefsMetadataDedupClosure cl(q, dq, rp);
-        mark_loop_work<ShenandoahMarkRefsMetadataDedupClosure, CANCELLABLE, DRAIN_SATB>(&cl, ld, w, t);
+        mark_loop_work<ShenandoahMarkRefsMetadataDedupClosure, CANCELLABLE>(&cl, ld, w, t);
       } else {
         ShenandoahMarkRefsMetadataClosure cl(q, rp);
-        mark_loop_work<ShenandoahMarkRefsMetadataClosure, CANCELLABLE, DRAIN_SATB>(&cl, ld, w, t);
+        mark_loop_work<ShenandoahMarkRefsMetadataClosure, CANCELLABLE>(&cl, ld, w, t);
       }
     }
   } else {
     if (update_refs) {
       if (strdedup) {
         ShenandoahStrDedupQueue* dq = ShenandoahStringDedup::queue(w);
         ShenandoahMarkUpdateRefsDedupClosure cl(q, dq, rp);
-        mark_loop_work<ShenandoahMarkUpdateRefsDedupClosure, CANCELLABLE, DRAIN_SATB>(&cl, ld, w, t);
+        mark_loop_work<ShenandoahMarkUpdateRefsDedupClosure, CANCELLABLE>(&cl, ld, w, t);
       } else {
         ShenandoahMarkUpdateRefsClosure cl(q, rp);
-        mark_loop_work<ShenandoahMarkUpdateRefsClosure, CANCELLABLE, DRAIN_SATB>(&cl, ld, w, t);
+        mark_loop_work<ShenandoahMarkUpdateRefsClosure, CANCELLABLE>(&cl, ld, w, t);
       }
     } else {
       if (strdedup) {
         ShenandoahStrDedupQueue* dq = ShenandoahStringDedup::queue(w);
         ShenandoahMarkRefsDedupClosure cl(q, dq, rp);
-        mark_loop_work<ShenandoahMarkRefsDedupClosure, CANCELLABLE, DRAIN_SATB>(&cl, ld, w, t);
+        mark_loop_work<ShenandoahMarkRefsDedupClosure, CANCELLABLE>(&cl, ld, w, t);
       } else {
         ShenandoahMarkRefsClosure cl(q, rp);
-        mark_loop_work<ShenandoahMarkRefsClosure, CANCELLABLE, DRAIN_SATB>(&cl, ld, w, t);
+        mark_loop_work<ShenandoahMarkRefsClosure, CANCELLABLE>(&cl, ld, w, t);
       }
     }
   }
 
 

@@ -954,11 +944,11 @@
       r->increase_live_data_gc_words(live);
     }
   }
 }
 
-template <class T, bool CANCELLABLE, bool DRAIN_SATB>
+template <class T, bool CANCELLABLE>
 void ShenandoahConcurrentMark::mark_loop_work(T* cl, jushort* live_data, uint worker_id, ParallelTaskTerminator *terminator) {
   int seed = 17;
   uintx stride = CANCELLABLE ? ShenandoahMarkLoopStride : 1;
 
   ShenandoahHeap* heap = ShenandoahHeap::heap();

@@ -1007,15 +997,13 @@
       ShenandoahCancelledTerminatorTerminator tt;
       while (!terminator->offer_termination(&tt));
       return;
     }
 
-    if (DRAIN_SATB) {
       while (satb_mq_set.completed_buffers_num() > 0) {
         satb_mq_set.apply_closure_to_completed_buffer(&drain_satb);
       }
-    }
 
     uint work = 0;
     for (uint i = 0; i < stride; i++) {
       if (try_queue(q, t) ||
           queues->steal(worker_id, &seed, t)) {