RFR 8225483: Shenandoah: Enhance native access barrier

[Roman Kennke]

I think it returns either NULL or passes through LRB,  I think this is fine and does not leak from-space-refs.

Roman


Am 11. Juni 2019 11:23:12 MESZ schrieb Aleksey Shipilev <shade at redhat.com>:
>On 6/11/19 1:14 AM, Zhengyu Gu wrote:
>> On 6/10/19 4:32 AM, Aleksey Shipilev wrote:
>>> On 6/10/19 2:06 AM, Zhengyu Gu wrote:
>>>> Please review this enhancement of native access barrier, in
>preparation for concurrent root
>>>> processing.
>>>>
>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8225483
>>>> Webrev: http://cr.openjdk.java.net/~zgu/JDK-8225483/webrev.00/
>>>
>>> *) It looks to me the webrev itself and the patch in it disagree.
>For example, webrev says the
>>> entire oop_store_not_in_heap is #ifdef ASSERT-ed, but the patch has
>the ASSERT block internally. The
>>> patch also has the unrelated change in shenandoahForwarding.hpp.
>> 
>> Sorry, it is a broken webrev.
>> 
>> Updated: http://cr.openjdk.java.net/~zgu/JDK-8225483/webrev.01/
>
>This patch cannot go to jdk/jdk. It can only go to sh/jdk.
>
>*) This block:
>
>  99   ShenandoahHeap* const heap = ShenandoahHeap::heap();
>100   shenandoah_assert_marked_if(NULL, value,
>!CompressedOops::is_null(value) &&
>heap->is_evacuation_in_progress());
>
>Can be just:
>
>shenandoah_assert_marked_if(NULL, value,
>!CompressedOops::is_null(value) &&
>ShenandoahHeap::heap()->is_evacuation_in_progress());
>
>*) After the change above is done, you can make
>SBS::oop_store_not_in_heap available always: e.g.
>drop the ASSERT around it.
>
>*) I think the block in oop_load_not_in_heap needs the comment why this
>is done. This is a weird
>exception anyway: it leaks the from-space pointers from the weak roots,
>no? What actually guarantees
>we would not store that from-space pointer somewhere *on heap*?
>
>*) This block can be simplified:
>
>   ShenandoahHeap* const heap = ShenandoahHeap::heap();
>   if (heap->is_evacuation_in_progress()) {
>ShenandoahMarkingContext* const marking_context =
>heap->complete_marking_context();
>     if (!marking_context->is_marked(value)) {
>       return NULL;
>     }
>   }
>
>to:
>
>   ShenandoahHeap* const heap = ShenandoahHeap::heap();
>if (heap->is_evacuation_in_progress() &&
>!heap->complete_marking_context()->is_marked(value)) {
>     return NULL;
>   }
>
>-Aleksey

-- 
Diese Nachricht wurde von meinem Android-Gerät mit K-9 Mail gesendet.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20190611/3def3877/attachment.htm>