RFR 8221435: Shenandoah should not mark through weak roots

Roman Kennke rkennke at redhat.com
Tue Mar 26 19:31:10 UTC 2019



>> *) I think the WeakProcessor::weak_oops_do is deliberately called with 
>> serial version when
>> ShenandoahWeakAssertNotForwardedClosure is handled. Maybe that 
>> prolongs weak root handling
>> unnecessarily? Anyhow, this comment needs to be moved to new code:
> 
> I was puzzled by this also. It may be the case in early days, when there 
> are few oop storages? seems we definitely can benefit parallel version, 
> because there are more oop storages to process.
> 
> Roman, do you recall the reason why it was done this way?
> 
> ShenandoahWeakAssertNotForwardedClosure is debug only closure, surround 
> the body with ifdef.

Well, as you say, it's debug-only, so shouldn't matter performance-wise. 
Except, as you say, it should be #ifdef ASSERT. It wasn't #ifdef'd 
before? WTF?

Other than that, looks good. Maybe we should wait until we have fixes for:

https://bugs.openjdk.java.net/browse/JDK-8221102
and
https://bugs.openjdk.java.net/browse/JDK-8220671

in sh/jdk, so that we have a stable testing base?

Roman

>>   709     // Process leftover weak oops: update them, if needed (using 
>> parallel version),
>>   710     // or assert they do not need updating (using serial 
>> version) otherwise.
>>   711     // Weak processor API requires us to visit the oops, even if 
>> we are not doing
>>   712     // anything to them.
>>
>> *) The distinction between ShenandoahRootProcessor::process_all_roots, 
>> ::update_all_roots,
>> ::traversal_update_all_roots might be better? (I cannot see the better 
>> factoring right away, though).
>>
> There are inconsistencies in traversal, I am looking into them in follow 
> up RFE, will address this there.
> 
> Thanks,
> 
> -Zhengyu
> 
> 
>> -Aleksey
>>


More information about the hotspot-gc-dev mailing list