RFR: 8236981: Remove ShenandoahTraversalUpdateRefsClosure
Aleksey Shipilev
shade at redhat.com
Fri Mar 6 12:44:33 UTC 2020
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
More information about the shenandoah-dev
mailing list