RFR: 8241605: Shenandoah: More aggressive reference discovery

[Roman Kennke]

Am 26.03.20 um 10:14 schrieb 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! I pushed the change to shenandoah/jdk.

Roman