<html><head></head><body>It looks to me like it adds more work, while not reaping the benefit of it, specifically the ability to clean some roots concurrently. I assume this would come in follow-up?<br><br>Roman<br><br><br><div class="gmail_quote">Am 11. Juni 2019 01:14:48 MESZ schrieb Zhengyu Gu <zgu@redhat.com>:<blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
<pre class="k9mail"><br><br>On 6/10/19 4:32 AM, Aleksey Shipilev wrote:<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #729fcf; padding-left: 1ex;">On 6/10/19 2:06 AM, Zhengyu Gu wrote:<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #ad7fa8; padding-left: 1ex;"> Please review this enhancement of native access barrier, in preparation for concurrent root processing.<br><br> Bug: <a href="https://bugs.openjdk.java.net/browse/JDK-8225483">https://bugs.openjdk.java.net/browse/JDK-8225483</a><br> Webrev: <a href="http://cr.openjdk.java.net/~zgu/JDK-8225483/webrev.00/">http://cr.openjdk.java.net/~zgu/JDK-8225483/webrev.00/</a><br></blockquote><br>*) It looks to me the webrev itself and the patch in it disagree. For example, webrev says the<br>entire oop_store_not_in_heap is #ifdef ASSERT-ed, but the patch has the ASSERT block internally. The<br>patch also has the unrelated change in shenandoahForwarding.hpp.<br></blockquote><br>Sorry, it is a broken webrev.<br><br>Updated: <a href="http://cr.openjdk.java.net/~zgu/JDK-8225483/webrev.01/">http://cr.openjdk.java.net/~zgu/JDK-8225483/webrev.01/</a><br><br>Reran hotspot_gc_shenandoah (fastdebug and release)<br><br>Thanks,<br><br>-Zhengyu<br><br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #729fcf; padding-left: 1ex;"><br>*) Why change Raw::oop_store_in_heap to Raw::oop_store in ShenandoahBarrierSet::oop_store_in_heap?<br><br>*) You can write this block:<br><br>#ifdef ASSERT<br> ShenandoahHeap* const heap = ShenandoahHeap::heap();<br> shenandoah_assert_marked_if(value, heap->is_gc_in_progress_mask(ShenandoahHeap::EVACUATION));<br>#endif<br><br>as:<br><br> shenandoah_assert_marked_if(value, ShenandoahHeap::heap()->is_evacuation_in_progress());<br><br><br>*) Also, this block:<br><br> ShenandoahHeap* const heap = ShenandoahHeap::heap();<br> if (!CompressedOops::is_null(value) &&<br> heap->is_gc_in_progress_mask(ShenandoahHeap::EVACUATION)) {<br><br>as:<br><br> if (!CompressedOops::is_null(value) && ShenandoahHeap::heap()->is_evacuation_in_progress()) {<br><br><br><br>-Aleksey<br><br></blockquote></pre></blockquote></div><br>-- <br>Diese Nachricht wurde von meinem Android-Gerät mit K-9 Mail gesendet.</body></html>