RFR 8223774: Shenandoah: Refactor ShenandoahRootProcessor and family

Roman Kennke rkennke at redhat.com
Fri May 17 12:33:46 UTC 2019


>>> Updated: http://cr.openjdk.java.net/~zgu/JDK-8223774/webrev.02/

Looks good (with Aleksey's suggestion).

Thanks & cheers,
Roman

>>
>> I like it.
>>
>> *) No, this block should be in ShenandoahTraversalGC*Heuristics*, not
>> in ShenandoahTraversalGC
>> itself, look:
>> http://hg.openjdk.java.net/jdk/jdk/file/23837d614c17/src/hotspot/share/gc/shenandoah/heuristics/shenandoahTraversalHeuristics.cpp#l36
>>
>>
>>   313   // Traversal does not support concurrent code root scanning
>>   314   FLAG_SET_DEFAULT(ShenandoahConcurrentScanCodeRoots, false);
>>
>> *) One more naming suggestion (see if it makes sense!). Current
>> hierarchy is:
>>
>>   ShenandoahRootProcessingPhase
>>     -> ShenandoahRootProcessor
>>     -> ShenandoahRootEvacuator
>>     -> ShenandoahRootUpdater
>>     -> ShenandoahRootAdjuster
>>
>> It leads to awkward superclass name. Maybe we should rename
>> ShenandoahRootProcessor to reflect what
>> it does to also match Evacuator/Updater/Adjuster? This frees
>> ShenandoahRootProcessor to become the
>> superclass. For example:
>>
>>   ShenandoahRootProcessor
>>     -> ShenandoahRootScanner [Does it *only* scan, though? Dunno.]
>>     -> ShenandoahRootEvacuator
>>     -> ShenandoahRootUpdater
>>     -> ShenandoahRootAdjuster
> Was in my mind too, too lazy to rename :-( will do before push.
> 
> Thanks,
> 
> -Zhengyu
> 
>>
>> -Aleksey
>>
>>



More information about the shenandoah-dev mailing list