RFR: 8236981: Remove ShenandoahTraversalUpdateRefsClosure

[Aditya Mandaleeka]

Thank you Aleksey and Roman for reviewing. Please let me know if you'd like me to help with backporting
this change or anything.

Thanks,
Aditya

-----Original Message-----
From: Aleksey Shipilev <shade at redhat.com> 
Sent: Friday, March 6, 2020 4:45 AM
To: Roman Kennke <rkennke at redhat.com>; Aditya Mandaleeka <adityam at microsoft.com>; shenandoah-dev <shenandoah-dev at openjdk.java.net>
Subject: Re: RFR: 8236981: Remove ShenandoahTraversalUpdateRefsClosure

On 3/6/20 1:31 PM, Roman Kennke wrote:
>> On 3/6/20 7:37 AM, Aditya Mandaleeka wrote:
>>> diff -r 92cf8efd381d src/hotspot/share/gc/shenandoah/shenandoahTraversalGC.cpp
>>> --- a/src/hotspot/share/gc/shenandoah/shenandoahTraversalGC.cpp Fri Mar 06 10:27:24 2020 +0530
>>> +++ b/src/hotspot/share/gc/shenandoah/shenandoahTraversalGC.cpp Thu Mar 05 22:30:55 2020 -0800
>>> @@ -605,8 +605,11 @@
>>>      // that results the TLAB/GCLAB not usable. Retire them here.
>>>      _heap->make_parsable(true);
>>>
>>> +    // Do this fixup before the call to parallel_cleaning to ensure that all
>>> +    // forwarded objects (including those that are no longer in the cset) are
>>> +    // updated by the time we do weak root processing.
>>> +    fixup_roots();
>>>      _heap->parallel_cleaning(false);
>>> -    fixup_roots();
>>>
>>>      _heap->set_has_forwarded_objects(false);
>>
>> ...but this thing reverts recent fix!
>>   https://hg.openjdk.java.net/jdk/jdk/rev/b997e5b9479b#l3.1
>>
>> I wonder what is up with that, Roman?
> 
> This code is a mess. It doesn't really matter if we fix the roots first
> and then clean them, or the other way around. In the other fix I thought
> that it would be more efficient to clean them first and then update what
> remains. But that causes troubles on the degen-path because we get there
> with cset down.
> 
> Ideally we should clean what is unreachable and update the rest in one
> pass, this requires some work though, which is not the scope of this change.
> 
> To me the change looks good. I can sponsor it.

Okay then!

-- 
Thanks,
-Aleksey