RFR 8221435: Shenandoah should not mark through weak roots
Aleksey Shipilev
shade at redhat.com
Tue Mar 26 17:21:34 UTC 2019
On 3/26/19 6:05 PM, Zhengyu Gu wrote:
> Please review this patch that fixes incorrectly marking through weak roots in Shenandoah.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8221435
> Webrev: http://cr.openjdk.java.net/~zgu/JDK-8221435/webrev.00/index.html
*) Please qrefresh the changeset comment
*) shenandoahConcurrentMark.cpp, indenting at L660..L661
659 if (_heap->has_forwarded_objects()) {
660 ShenandoahWeakUpdateClosure cl;
661 WeakProcessor::weak_oops_do(workers, is_alive.is_alive_closure(), &cl, 1);
662 } else {
663 ShenandoahWeakAssertNotForwardedClosure cl;
664 WeakProcessor::weak_oops_do(workers, is_alive.is_alive_closure(), &cl, 1);
665 }
*) 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:
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).
-Aleksey
More information about the shenandoah-dev
mailing list