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

[Aleksey Shipilev]

On 06/19/2018 12:09 PM, Roman Kennke wrote:
> 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
>   }

Ah, I missed this part: we actually return task.start() == NULL on this path. Okay then.

-Aleksey