RFR: 8236981: Remove ShenandoahTraversalUpdateRefsClosure

Roman Kennke rkennke at redhat.com
Fri Mar 6 12:31:49 UTC 2020




> The removal looks okay.
> 
> 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.

Thank you, Aditya!

Roman



More information about the shenandoah-dev mailing list