RFR: Improve scheduling and interleaving of SATB processing in mark loop

[Aleksey Shipilev]

On 06/18/2018 10:17 AM, Roman Kennke wrote:
> Am 18.06.2018 um 09:58 schrieb Roman Kennke:
>> Printing out when SATB buffers get enqueued (by mutators) and processed
>> (by collectors) shows that the way we currently process SATB buffers
>> (only when marking stack runs empty) tends to process SATB buffers
>> towards the end of marking, thus piling up SATB buffers throughout most
>> of the marking cycle.
>>
>> I propose to also process SATB buffers (if available) between marking
>> strides. This leads to more promptly processing SATB buffers, thus
>> smoothing out SATB pressure during marking cycle. I also added an
>> os::naked_yield() between strides to give mutators a chance to run.
>>
>> http://cr.openjdk.java.net/~rkennke/interleave-satb/webrev.00/
>>
>> Testing: tier3_gc_shenandoah
>>
>> Good? WDYT?
>>
>> Roman
>>
> 
> I am getting test failures. Don't know yet, why, but will investigate.
> Putting the RFR on hold.

1021     if (DRAIN_SATB) {
1022       try_draining_satb_buffer(q, t);
1023     }

You have lost "t" here: it is actually the task we need to do_task afterwards, otherwise you are
missing objects to mark.

But the locking problem still stands. I think the whole loop should be like:

  while (true) {
    if (CANCELLABLE && heap->check_cancelled_gc_and_yield()) {
      ShenandoahCancelledTerminatorTerminator tt;
      while (!terminator->offer_termination(&tt));
      return;
    }

    if (DRAIN_SATB) {
      SATBMarkQueueSet& satb_mq_set = ShenandoahBarrierSet::satb_mark_queue_set();
      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)) {
        work++;
        do_task<T>(q, cl, live_data, &t);
      }
    }

    if (work == 0) {
      // No work encountered in current stride, try to terminate.
      // Need to leave the STS here otherwise it might block safepoints.
      SuspendibleThreadSetLeaver stsl(CANCELLABLE && ShenandoahSuspendibleWorkers);
      if (terminator->offer_termination()) return;
    }
  }

I can work the whole patch out performance-wise.


-Aleksey