RFR: Implement arraycopy post-processing in GC threads for Traversal GC

Roman Kennke rkennke at redhat.com
Tue Jun 19 10:09:13 UTC 2018


Am 19.06.2018 um 12:04 schrieb Aleksey Shipilev:
> On 06/19/2018 11:22 AM, Roman Kennke wrote:
>> It turns out that we do have to do the NULL-check. Checking for length
>> is racy, doing it non-race means we'd have to take the lock every time
>> we go there. It seems better to make a racy check, and then return NULL
>> if queue is indeed empty, and handle it specially.
>>
>> Incremental:
>> http://cr.openjdk.java.net/~rkennke/traversal-arraycopy/webrev.03.diff/
> 
> Um. I don't understand how checking for (task.start() == NULL) resolves the race. Actually, I do not
> understand this issue at all. There is a preceding _arraycopy_task_queue.pop(), which asserts
> internally that (task.start() != NULL)!
> 
> 1175   ShenandoahArrayCopyTask task = _arraycopy_task_queue.pop();
> 1176   if (task.start() == NULL) {
> 1177     return false;
> 1178   }
> 
>   53 ShenandoahArrayCopyTask ShenandoahArrayCopyTaskQueue::pop() {
>        ...
>   58   ShenandoahArrayCopyTask task = _buffer->pop();
>   59   assert(task.start() != NULL, "only non-NULL tasks in queue");  <--- !
>   60   return task;
>   61 }
> 
> -Aleksey
> 

ShenandoahArrayCopyTask ShenandoahArrayCopyTaskQueue::pop() {
  MutexLockerEx locker(_lock, Mutex::_no_safepoint_check_flag);
  if (_buffer->length() == 0) {
    return ShenandoahArrayCopyTask(NULL); // no-more-work-indicator
  }


We first check, under race, the length before calling
process_arraycopy_task():

    if (_arraycopy_task_queue.length() > 0) {
      process_arraycopy_task(cl);
    }

We might have 1 task left, and 2 threads going there anyway:

  ShenandoahArrayCopyTask task = _arraycopy_task_queue.pop();
  if (task.start() == NULL) {
    return false;
  }

pop() checks length again, but this time under lock, and return a NULL
task, which is checked and handled upon by process_arraycopy_task():

  if (task.start() == NULL) {
    return false;
  }


Please suggest a better/easier-to-understand scheme!

Thanks,
Roman



More information about the shenandoah-dev mailing list