RFR 8225483: Shenandoah: Enhance native access barrier

[Aleksey Shipilev]

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20190611/e5c4fae0/signature.asc>