RFR (XS): ConcurrentMark overflow queues cleanup

[Aleksey Shipilev]

Hi,

So, during the debugging session we discovered the inconsistency between
ShenandoahConcurrentMark::concurrent_mark_loop and
ShenandoahConcurrentMark::cancel. The first one does not empty the
overflow queues on cancelled concgc, which may set them up for
processing on the next cycle! Oops.

Also, % _max_conc_worker_id is not safe if we want to maintain the
invariant that any given queue is only served by a single thread.

Fixes:

diff -r f0002694db11 src/share/vm/gc/shenandoah/shenandoahConcurrentMark.hpp
--- a/src/share/vm/gc/shenandoah/shenandoahConcurrentMark.hpp	Tue Nov 08
15:50:02 2016 +0100
+++ b/src/share/vm/gc/shenandoah/shenandoahConcurrentMark.hpp	Tue Nov 08
22:41:27 2016 +0100
@@ -145,6 +145,8 @@
   inline bool try_to_steal(uint worker_id, ShenandoahMarkObjsClosure<T,
CL>* cl, int *seed);

   SCMObjToScanQueue* get_queue(uint worker_id);
+  void clear_queue(SCMObjToScanQueue *q);
+
   inline bool try_draining_an_satb_buffer(SCMObjToScanQueue* q);
   void drain_satb_buffers(uint worker_id, bool remark = false);
   SCMObjToScanQueueSet* task_queues() { return _task_queues;}

diff -r f0002694db11 src/share/vm/gc/shenandoah/shenandoahConcurrentMark.cpp
--- a/src/share/vm/gc/shenandoah/shenandoahConcurrentMark.cpp	Tue Nov 08
15:50:02 2016 +0100
+++ b/src/share/vm/gc/shenandoah/shenandoahConcurrentMark.cpp	Tue Nov 08
22:45:47 2016 +0100
@@ -857,19 +857,24 @@
   // Clean up marking stacks.
   SCMObjToScanQueueSet* queues = task_queues();
   for (uint i = 0; i < _max_conc_worker_id; ++i) {
-    SCMObjToScanQueue* task_queue = queues->queue(i);
-    task_queue->set_empty();
-    task_queue->overflow_stack()->clear();
+    clear_queue(queues->queue(i));
   }

   // Cancel SATB buffers.
   JavaThread::satb_mark_queue_set().abandon_partial_marking();
 }
+
 SCMObjToScanQueue* ShenandoahConcurrentMark::get_queue(uint worker_id) {
-  worker_id = worker_id % _max_conc_worker_id;
+  assert (worker_id < _max_conc_worker_id, "valid worker id: %d",
worker_id);
   return _task_queues->queue(worker_id);
 }

+void ShenandoahConcurrentMark::clear_queue(SCMObjToScanQueue *q) {
+  q->set_empty();
+  q->overflow_stack()->clear();
+}
+
 template <class T, bool CL>
 void
ShenandoahConcurrentMark::concurrent_mark_loop(ShenandoahMarkObjsClosure<T,
CL>* cl,
                                                     uint worker_id,
@@ -878,7 +883,7 @@
   ShenandoahHeap* heap = ShenandoahHeap::heap();
   int seed = 17;
   while (true) {
-    if (heap->cancelled_concgc())  q->set_empty();
+    if (heap->cancelled_concgc()) clear_queue(q);
     if (heap->cancelled_concgc() ||
         (!try_queue(q, cl) &&

Good to go?

Thanks,
-Aleksey