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