# HG changeset patch
# User rkennke
# Date 1529434989 -7200
#      Tue Jun 19 21:03:09 2018 +0200
# Node ID de36d073d4aaac18b8700fb91d3358ac15765e7e
# Parent  a6db1faa4776a924f0f4a0d7cf9bd9998889057a
Process remaining SATB buffers in final mark/traverse loop instead of separate phase

diff --git a/src/hotspot/share/gc/shenandoah/shenandoahConcurrentMark.cpp b/src/hotspot/share/gc/shenandoah/shenandoahConcurrentMark.cpp
--- a/src/hotspot/share/gc/shenandoah/shenandoahConcurrentMark.cpp
+++ b/src/hotspot/share/gc/shenandoah/shenandoahConcurrentMark.cpp
@@ -205,7 +205,6 @@
     _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
@@ -233,7 +232,12 @@
     // 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);
+    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));
+    ShenandoahSATBThreadsClosure tc(&cl);
+    Threads::threads_do(&tc);
 
     ReferenceProcessor* rp;
     if (_cm->process_references()) {
@@ -248,7 +252,6 @@
     _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);
@@ -499,19 +502,6 @@
   }
 };
 
-void ShenandoahConcurrentMark::drain_satb_buffers(uint worker_id, bool remark) {
-  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");
@@ -575,7 +565,6 @@
 
     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
@@ -801,7 +790,6 @@
 
     scm->mark_loop(0, &terminator, rp,
                    false, // not cancellable
-                   true,  // drain SATBs
                    scm->unload_classes(),
                    sh->has_forwarded_objects(),
                    false); // do not do strdedup
@@ -894,7 +882,7 @@
   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);
@@ -909,19 +897,19 @@
       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 {
@@ -929,19 +917,19 @@
       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);
       }
     }
   }
@@ -956,7 +944,7 @@
   }
 }
 
-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;
@@ -1009,10 +997,8 @@
       return;
     }
 
-    if (DRAIN_SATB) {
-      while (satb_mq_set.completed_buffers_num() > 0) {
-        satb_mq_set.apply_closure_to_completed_buffer(&drain_satb);
-      }
+    while (satb_mq_set.completed_buffers_num() > 0) {
+      satb_mq_set.apply_closure_to_completed_buffer(&drain_satb);
     }
 
     uint work = 0;
diff --git a/src/hotspot/share/gc/shenandoah/shenandoahConcurrentMark.hpp b/src/hotspot/share/gc/shenandoah/shenandoahConcurrentMark.hpp
--- a/src/hotspot/share/gc/shenandoah/shenandoahConcurrentMark.hpp
+++ b/src/hotspot/share/gc/shenandoah/shenandoahConcurrentMark.hpp
@@ -66,10 +66,10 @@
   inline void count_liveness_humongous(oop obj);
 
   // Actual mark loop with closures set up
-  template <class T, bool CANCELLABLE, bool DRAIN_SATB>
+  template <class T, bool CANCELLABLE>
   void mark_loop_work(T* cl, jushort* live_data, uint worker_id, ParallelTaskTerminator *t);
 
-  template <bool CANCELLABLE, bool DRAIN_SATB>
+  template <bool CANCELLABLE>
   void mark_loop_prework(uint worker_id, ParallelTaskTerminator *terminator, ReferenceProcessor *rp,
                          bool class_unload, bool update_refs, bool strdedup);
 
@@ -77,20 +77,12 @@
   // Mark loop entry.
   // Translates dynamic arguments to template parameters with progressive currying.
   void mark_loop(uint worker_id, ParallelTaskTerminator* terminator, ReferenceProcessor *rp,
-                 bool cancellable, bool drain_satb,
+                 bool cancellable,
                  bool class_unload, bool update_refs, bool strdedup) {
     if (cancellable) {
-      if (drain_satb) {
-        mark_loop_prework<true, true>(worker_id, terminator, rp, class_unload, update_refs, strdedup);
-      } else {
-        mark_loop_prework<true, false>(worker_id, terminator, rp, class_unload, update_refs, strdedup);
-      }
+      mark_loop_prework<true>(worker_id, terminator, rp, class_unload, update_refs, strdedup);
     } else {
-      if (drain_satb) {
-        mark_loop_prework<false, true>(worker_id, terminator, rp, class_unload, update_refs, strdedup);
-      } else {
-        mark_loop_prework<false, false>(worker_id, terminator, rp, class_unload, update_refs, strdedup);
-      }
+      mark_loop_prework<false>(worker_id, terminator, rp, class_unload, update_refs, strdedup);
     }
   }
 
@@ -126,7 +118,6 @@
   ShenandoahObjToScanQueue* get_queue(uint worker_id);
   void clear_queue(ShenandoahObjToScanQueue *q);
 
-  void drain_satb_buffers(uint worker_id, bool remark = false);
   ShenandoahObjToScanQueueSet* task_queues() { return _task_queues;}
 
   jushort* get_liveness(uint worker_id);
diff --git a/src/hotspot/share/gc/shenandoah/shenandoahTraversalGC.cpp b/src/hotspot/share/gc/shenandoah/shenandoahTraversalGC.cpp
--- a/src/hotspot/share/gc/shenandoah/shenandoahTraversalGC.cpp
+++ b/src/hotspot/share/gc/shenandoah/shenandoahTraversalGC.cpp
@@ -213,7 +213,7 @@
     ShenandoahTraversalGC* traversal_gc = _heap->traversal_gc();
 
     // Drain all outstanding work in queues.
-    traversal_gc->main_loop(worker_id, _terminator, true);
+    traversal_gc->main_loop(worker_id, _terminator);
   }
 };
 
@@ -243,7 +243,7 @@
       rp = _heap->ref_processor();
     }
 
-    // Step 1: Drain outstanding SATB queues.
+    // Step 0: Drain outstanding SATB queues.
     // NOTE: we piggy-back draining of remaining thread SATB buffers on the final root scan below.
     ShenandoahTraversalSATBBufferClosure satb_cl(q);
     {
@@ -283,7 +283,7 @@
       ShenandoahWorkerTimingsTracker timer(worker_times, ShenandoahPhaseTimings::FinishQueues, worker_id);
 
       // Step 3: Finally drain all outstanding work in queues.
-      traversal_gc->main_loop(worker_id, _terminator, false);
+      traversal_gc->main_loop(worker_id, _terminator);
     }
 
   }
@@ -435,16 +435,7 @@
   _root_regions_iterator.reset(_root_regions);
 }
 
-void ShenandoahTraversalGC::main_loop(uint worker_id, ParallelTaskTerminator* terminator, bool do_satb) {
-  if (do_satb) {
-    main_loop_prework<true>(worker_id, terminator);
-  } else {
-    main_loop_prework<false>(worker_id, terminator);
-  }
-}
-
-template <bool DO_SATB>
-void ShenandoahTraversalGC::main_loop_prework(uint w, ParallelTaskTerminator* t) {
+void ShenandoahTraversalGC::main_loop(uint w, ParallelTaskTerminator* t) {
   ShenandoahObjToScanQueue* q = task_queues()->queue(w);
 
   // Initialize live data.
@@ -461,19 +452,19 @@
         if (ShenandoahStringDedup::is_enabled()) {
           ShenandoahStrDedupQueue* dq = ShenandoahStringDedup::queue(w);
           ShenandoahTraversalMetadataDedupMatrixClosure cl(q, rp, dq);
-          main_loop_work<ShenandoahTraversalMetadataDedupMatrixClosure, DO_SATB>(&cl, ld, w, t);
+          main_loop_work<ShenandoahTraversalMetadataDedupMatrixClosure>(&cl, ld, w, t);
         } else {
           ShenandoahTraversalMetadataMatrixClosure cl(q, rp);
-          main_loop_work<ShenandoahTraversalMetadataMatrixClosure, DO_SATB>(&cl, ld, w, t);
+          main_loop_work<ShenandoahTraversalMetadataMatrixClosure>(&cl, ld, w, t);
         }
       } else {
         if (ShenandoahStringDedup::is_enabled()) {
           ShenandoahStrDedupQueue* dq = ShenandoahStringDedup::queue(w);
           ShenandoahTraversalDedupMatrixClosure cl(q, rp, dq);
-          main_loop_work<ShenandoahTraversalDedupMatrixClosure, DO_SATB>(&cl, ld, w, t);
+          main_loop_work<ShenandoahTraversalDedupMatrixClosure>(&cl, ld, w, t);
         } else {
           ShenandoahTraversalMatrixClosure cl(q, rp);
-          main_loop_work<ShenandoahTraversalMatrixClosure, DO_SATB>(&cl, ld, w, t);
+          main_loop_work<ShenandoahTraversalMatrixClosure>(&cl, ld, w, t);
         }
       }
     } else {
@@ -481,19 +472,19 @@
         if (ShenandoahStringDedup::is_enabled()) {
           ShenandoahStrDedupQueue* dq = ShenandoahStringDedup::queue(w);
           ShenandoahTraversalMetadataDedupDegenMatrixClosure cl(q, rp, dq);
-          main_loop_work<ShenandoahTraversalMetadataDedupDegenMatrixClosure, DO_SATB>(&cl, ld, w, t);
+          main_loop_work<ShenandoahTraversalMetadataDedupDegenMatrixClosure>(&cl, ld, w, t);
         } else {
           ShenandoahTraversalMetadataDegenMatrixClosure cl(q, rp);
-          main_loop_work<ShenandoahTraversalMetadataDegenMatrixClosure, DO_SATB>(&cl, ld, w, t);
+          main_loop_work<ShenandoahTraversalMetadataDegenMatrixClosure>(&cl, ld, w, t);
         }
       } else {
         if (ShenandoahStringDedup::is_enabled()) {
           ShenandoahStrDedupQueue* dq = ShenandoahStringDedup::queue(w);
           ShenandoahTraversalDedupDegenMatrixClosure cl(q, rp, dq);
-          main_loop_work<ShenandoahTraversalDedupDegenMatrixClosure, DO_SATB>(&cl, ld, w, t);
+          main_loop_work<ShenandoahTraversalDedupDegenMatrixClosure>(&cl, ld, w, t);
         } else {
           ShenandoahTraversalDegenMatrixClosure cl(q, rp);
-          main_loop_work<ShenandoahTraversalDegenMatrixClosure, DO_SATB>(&cl, ld, w, t);
+          main_loop_work<ShenandoahTraversalDegenMatrixClosure>(&cl, ld, w, t);
         }
       }
     }
@@ -503,19 +494,19 @@
         if (ShenandoahStringDedup::is_enabled()) {
           ShenandoahStrDedupQueue* dq = ShenandoahStringDedup::queue(w);
           ShenandoahTraversalMetadataDedupClosure cl(q, rp, dq);
-          main_loop_work<ShenandoahTraversalMetadataDedupClosure, DO_SATB>(&cl, ld, w, t);
+          main_loop_work<ShenandoahTraversalMetadataDedupClosure>(&cl, ld, w, t);
         } else {
           ShenandoahTraversalMetadataClosure cl(q, rp);
-          main_loop_work<ShenandoahTraversalMetadataClosure, DO_SATB>(&cl, ld, w, t);
+          main_loop_work<ShenandoahTraversalMetadataClosure>(&cl, ld, w, t);
         }
       } else {
         if (ShenandoahStringDedup::is_enabled()) {
           ShenandoahStrDedupQueue* dq = ShenandoahStringDedup::queue(w);
           ShenandoahTraversalDedupClosure cl(q, rp, dq);
-          main_loop_work<ShenandoahTraversalDedupClosure, DO_SATB>(&cl, ld, w, t);
+          main_loop_work<ShenandoahTraversalDedupClosure>(&cl, ld, w, t);
         } else {
           ShenandoahTraversalClosure cl(q, rp);
-          main_loop_work<ShenandoahTraversalClosure, DO_SATB>(&cl, ld, w, t);
+          main_loop_work<ShenandoahTraversalClosure>(&cl, ld, w, t);
         }
       }
     } else {
@@ -523,19 +514,19 @@
         if (ShenandoahStringDedup::is_enabled()) {
           ShenandoahStrDedupQueue* dq = ShenandoahStringDedup::queue(w);
           ShenandoahTraversalMetadataDedupDegenClosure cl(q, rp, dq);
-          main_loop_work<ShenandoahTraversalMetadataDedupDegenClosure, DO_SATB>(&cl, ld, w, t);
+          main_loop_work<ShenandoahTraversalMetadataDedupDegenClosure>(&cl, ld, w, t);
         } else {
           ShenandoahTraversalMetadataDegenClosure cl(q, rp);
-          main_loop_work<ShenandoahTraversalMetadataDegenClosure, DO_SATB>(&cl, ld, w, t);
+          main_loop_work<ShenandoahTraversalMetadataDegenClosure>(&cl, ld, w, t);
         }
       } else {
         if (ShenandoahStringDedup::is_enabled()) {
           ShenandoahStrDedupQueue* dq = ShenandoahStringDedup::queue(w);
           ShenandoahTraversalDedupDegenClosure cl(q, rp, dq);
-          main_loop_work<ShenandoahTraversalDedupDegenClosure, DO_SATB>(&cl, ld, w, t);
+          main_loop_work<ShenandoahTraversalDedupDegenClosure>(&cl, ld, w, t);
         } else {
           ShenandoahTraversalDegenClosure cl(q, rp);
-          main_loop_work<ShenandoahTraversalDegenClosure, DO_SATB>(&cl, ld, w, t);
+          main_loop_work<ShenandoahTraversalDegenClosure>(&cl, ld, w, t);
         }
       }
     }
@@ -544,7 +535,7 @@
 
 }
 
-template <class T, bool DO_SATB>
+template <class T>
 void ShenandoahTraversalGC::main_loop_work(T* cl, jushort* live_data, uint worker_id, ParallelTaskTerminator* terminator) {
   ShenandoahObjToScanQueueSet* queues = task_queues();
   ShenandoahObjToScanQueue* q = queues->queue(worker_id);
@@ -604,10 +595,8 @@
   while (true) {
     if (check_and_handle_cancelled_gc(terminator)) return;
 
-    if (DO_SATB) {
-      while (satb_mq_set.completed_buffers_num() > 0) {
-        satb_mq_set.apply_closure_to_completed_buffer(&drain_satb);
-      }
+    while (satb_mq_set.completed_buffers_num() > 0) {
+      satb_mq_set.apply_closure_to_completed_buffer(&drain_satb);
     }
 
     if (_arraycopy_task_queue.length() > 0) {
@@ -840,7 +829,7 @@
     assert(sh->process_references(), "why else would we be here?");
     ParallelTaskTerminator terminator(1, traversal_gc->task_queues());
     shenandoah_assert_rp_isalive_installed();
-    traversal_gc->main_loop((uint) 0, &terminator, false);
+    traversal_gc->main_loop((uint) 0, &terminator);
   }
 };
 
@@ -990,7 +979,7 @@
     assert(sh->process_references(), "why else would we be here?");
     shenandoah_assert_rp_isalive_installed();
 
-    traversal_gc->main_loop(_worker_id, _terminator, false);
+    traversal_gc->main_loop(_worker_id, _terminator);
 
     if (_reset_terminator) {
       _terminator->reset_for_reuse();
diff --git a/src/hotspot/share/gc/shenandoah/shenandoahTraversalGC.hpp b/src/hotspot/share/gc/shenandoah/shenandoahTraversalGC.hpp
--- a/src/hotspot/share/gc/shenandoah/shenandoahTraversalGC.hpp
+++ b/src/hotspot/share/gc/shenandoah/shenandoahTraversalGC.hpp
@@ -81,7 +81,7 @@
   jushort* get_liveness(uint worker_id);
   void flush_liveness(uint worker_id);
 
-  void main_loop(uint worker_id, ParallelTaskTerminator* terminator, bool do_satb);
+  void main_loop(uint worker_id, ParallelTaskTerminator* terminator);
 
   void push_arraycopy(HeapWord* start, size_t count);
 
@@ -89,10 +89,7 @@
 
   void prepare_regions();
 
-  template <bool DO_SATB>
-  void main_loop_prework(uint w, ParallelTaskTerminator* t);
-
-  template <class T, bool DO_SATB>
+  template <class T>
   void main_loop_work(T* cl, jushort* live_data, uint worker_id, ParallelTaskTerminator* terminator);
 
   void preclean_weak_refs();