RFR (XS): ConcurrentMark overflow queues cleanup

[Roman Kennke]

Yes, go!


Am Dienstag, den 08.11.2016, 22:46 +0100 schrieb 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(ShenandoahMarkObjsClos
> ure<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
> 
>