RFR: JDK-8231086: Shenandoah: Stronger invariant for object-arraycopy

Aleksey Shipilev shade at redhat.com
Wed Sep 18 10:50:30 UTC 2019


On 9/17/19 4:02 PM, Roman Kennke wrote:
> Webrev:
> https://bugs.openjdk.java.net/browse/JDK-8231086

The webrev seems to be actually here:
  http://cr.openjdk.java.net/~rkennke/JDK-8231086/webrev.00/

*) src/hotspot/cpu/aarch64/gc/shenandoah/shenandoahBarrierSetAssembler_aarch64.cpp

Indenting is bad at L63-65 here:

  60       if (dest_uninitialized) {
  61         __ tbz(rscratch2, ShenandoahHeap::HAS_FORWARDED_BITPOS, done);
  62       } else {
  63       __ mov(rscratch2, ShenandoahHeap::HAS_FORWARDED | ShenandoahHeap::MARKING);
  64       __ tst(rscratch1, rscratch2);
  65       __ br(Assembler::EQ, done);
  66       }

*) src/hotspot/cpu/x86/gc/shenandoah/shenandoahBarrierSetAssembler_x86.cpp

I think we can do this to simplify the code and expose x86_32 path early:

#ifdef _LP64
  assert(src == rdi, "expected");
  assert(dst == rsi, "expected");
  assert(count == rdx, "expected");
  if (UseCompressedOops) {
    if (dest_uninitialized) {
       ....call write_ref_array_pre_duinit_narrow_oop_entry
     } else {
       ... call write_ref_array_pre_narrow_oop_entry
     }
  } else
 #endif
  {
    if (dest_uninitialized) {
      ... call write_ref_array_pre_duinit_oop_entry
    } else {
      ... call write_ref_array_pre_oop_entry
    }
  }

*) src/hotspot/share/gc/shenandoah/c2/shenandoahBarrierSetC2.cpp

Do these locals serve any purpose?

  const char* copyfunc_name = "shenandoah_clone";
  address     copyfunc_addr = CAST_FROM_FN_PTR(address, ShenandoahRuntime::shenandoah_clone_barrier);
  const TypePtr* raw_adr_type = TypeRawPtr::BOTTOM;
  const TypeFunc* call_type = ShenandoahBarrierSetC2::shenandoah_clone_barrier_Type();
  Node* call = phase->make_leaf_call(ctrl, mem, call_type, copyfunc_addr, copyfunc_name,
raw_adr_type, src, dest, length);

Seems to be cleaner to inline them:

  Node* call = phase->make_leaf_call(ctrl, mem,
                  ShenandoahBarrierSetC2::shenandoah_clone_barrier_Type(),
                  CAST_FROM_FN_PTR(address, ShenandoahRuntime::shenandoah_clone_barrier),
                  "shenandoah_clone",
                  TypeRawPtr::BOTTOM,
                  src, dest, length);

*) src/hotspot/share/gc/shenandoah/shenandoahHeap.hpp

Having this one is a cute trick:
   static inline oop cas_oop(oop n, narrowOop* addr, narrowOop c);

Otherwise looks good to me.

-- 
Thanks,
-Aleksey



More information about the shenandoah-dev mailing list