RFR: 8241605: Shenandoah: More aggressive reference discovery

[Aleksey Shipilev]

On 3/26/20 9:49 AM, Roman Kennke wrote:
> Am 26.03.20 um 06:35 schrieb Aleksey Shipilev:
>> On 3/25/20 8:34 PM, Roman Kennke wrote:
>>>>> Webrev:
>>>>> http://cr.openjdk.java.net/~rkennke/JDK-8241605/webrev.00/
>>>>
>>>> *) I actually wonder if checking for TRAVERSAL|MARKING should be done as another generic
>>>> optimization? It seems good by itself to test for this instead of falling through for
>>>> evac/update-refs? (Actually, what checks for *that* in the old code?)
>>>
>>> I believe you're confusing the enqueue-barrier with the LRB here.
>>> EQ-barriers are only relevant during marking (and traversal).
>>
>> No, I am not.
>>
>> Look here in EQ barrier expansion:
>>
>>      // Stable path.
>> -    test_heap_stable(ctrl, raw_mem, heap_stable_ctrl, phase);
>> +    test_heap_stable(ctrl, raw_mem, heap_stable_ctrl, phase, ShenandoahHeap::TRAVERSAL |
>> ShenandoahHeap::MARKING);
>>      region->init_req(_heap_stable, heap_stable_ctrl);
>>      phi->init_req(_heap_stable, raw_mem);
>>
>>
>> In current code, EQ fastpath tests test_heap_stable (HAS_FORWARDED). In your patch, it now tests for
>> test_heap_state(TRAVERSAL|MARKING). Yes, EQ barriers are only relevant during marking/traversal, so
>> that must mean that _current code_ does test_heap_stable(HAS_FORWARDED) either incorrectly, or
>> inefficiently.
>>
>> This looks irrelevant to what aggressive reference discovery does?
> 
> Ah. Previously, we only used ShenandoahEnqueueBarrierNode with Traversal
> GC. There is only one phase, so the test for HAS_FORWARDED was actually
> testing for TRAVERSAL, but we took a short-cut there. Now we also use it
> for concurrent marking, so I couldn't take this short-cut anymore. The
> new code is correct.
> 
> Yes?

Ah, okay, that makes much more sense.

-- 
Thanks,
-Aleksey