RFR 8221435: Shenandoah should not mark through weak roots
zgu at redhat.com
Tue Mar 26 19:59:12 UTC 2019
On 3/26/19 3:31 PM, Roman Kennke wrote:
>>> *) 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:
Okay, I can hold off for them.
> in sh/jdk, so that we have a stable testing base?
>>> 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.
More information about the hotspot-gc-dev