RFR (sh/jdk8): Backport: JDK-8231086: Shenandoah: Stronger invariant for object-arraycopy

[Aleksey Shipilev]

On 2/20/20 9:41 PM, Roman Kennke wrote:
>>> http://cr.openjdk.java.net/~rkennke/JDK-8231086-jdk8/webrev.01/
>>
>> Okay, brief look:
>>
>> *) src/cpu/aarch64/vm/stubGenerator_aarch64.cpp
>>    src/cpu/x86/vm/stubGenerator_x86_32.cpp
>>
>>  Should new switch cases be protected by  #if INCLUDE_ALL_GCS too?
> 
> Good question. It wasn't before, and it isn't for the other GCs. Also,
> the enum that defines this doesn't have INCLUDE_ALL_GCS either. I think
> we might risk compiler complains about missing cases?

...also  src/cpu/x86/vm/stubGenerator_x86_64.cpp.

I think it works for other GCs, because they are calling the implementation from shared BarrierSet.
Without Shenandoah, shenandoahBarrierSetAssembler_*.hpp would not get included, so I would expect
call to ShenandoahBarrierSetAssembler::bsasm()->arraycopy_prologue(...) to break compilation.

>> *) I believe test is misplaced. Did that test even run?
>>   test/hotspot/jtreg/gc/shenandoah/compiler/TestClone.java
>> should be
>>   test/gc/shenandoah/compiler/TestClone.java
> 
> Oopsie. Right.

Test is still in the wrong place in webrev.02.

> http://cr.openjdk.java.net/~rkennke/JDK-8231086-jdk8/webrev.02/

Yes. Additional things after second read:

 *) Should be "#if INCLUDE_ALL_GCS", not "#ifdef INCLUDE_ALL_GCS". You probably need to #include
"utilities/macros.hpp" to gain access to it.

 *) Would you mind adding the comment to ShenandoahRuntime::shenandoah_clone_barrier, something like
(if that is correct):
  // In JDK 8u, caller does the arraycopy itself, and then calls the clone_barrier.
  // It is changed in later JDKs to do the copy right here, see JDK-????????

Otherwise seems fine.

-- 
Thanks,
-Aleksey