RFR: 8241605: Shenandoah: More aggressive reference discovery

[Roman Kennke]

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?

Thanks,
Roman