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