RFR: 8234974: Shenandoah: Do concurrent roots even when no evacuation is necessary

Zhengyu Gu zgu at redhat.com
Thu Dec 12 02:13:35 UTC 2019


Hi Roman,

This patch is a bit confusing, as it along, cleaning up weak roots 
during concurrent roots phase is unnecessary, because they are cleaned 
in parallel cleaning during final mark pause. Should it be part of 
concurrent weak root processing,  e.g. JDK-8228818?

Nit: probably should rename 
ShenandoahCodeRoots::prepare_concurrent_unloading() to
ShenandoahCodeRoots::arm_nmethods()

Otherwise, looks good.

Thanks,

-Zhengyu


On 12/11/19 5:12 PM, Roman Kennke wrote:
> After discussing off-line with Zhengyu, I am proposing those changes:
> 
> - Reinstate returning object for GC threads (apparently needed b/c of
> bug in JVMTI)
> - Always heal nmethods in barrier. We can only get there during evac
> (see below)
> - ShenandoahEvacUpdateOopStorageRootsClosure renamed to
> ShenandoahEvacUpdateCleanupRootsClosure and let it also handle cleanup.
> - Changed order of cleanup and unload: first do cleanup, then do unload.
> - only call prepare_unloading() when we have a cset. This arms nmethods.
> We only ever want to get into nmethod-barriers when we have a cset/are
> doing evacuation.
> 
> Incremental:
> http://cr.openjdk.java.net/~rkennke/JDK-8234974/webrev.01.diff/
> Full:
> http://cr.openjdk.java.net/~rkennke/JDK-8234974/webrev.01/
> 
> Good now?
> 
> Roman
> 
>> Shenandoah can short-cut a cycle when the collection set remains empty,
>> and doesn't dive into concurrent evacuation and updating refs phases
>> then. However, this currently also precludes concurrent roots processing
>> and concurrent class unloading. This is only a minor nuisance now
>> (effectively skipping class unloading for short-cut-cycles), but amounts
>> to a real bug when we're going to do weak-roots-cleaning concurrently too.
>>
>> Bug:
>> https://bugs.openjdk.java.net/browse/JDK-8234974
>> Webrev:
>> http://cr.openjdk.java.net/~rkennke/JDK-8234974/webrev.00/
>>
>> Testing: hotspot_gc_shenandoah
>>
>> Can I please get a review?
>>
>> Roman
>>
> 



More information about the shenandoah-dev mailing list