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

Roman Kennke rkennke at redhat.com
Fri Feb 21 12:13:41 UTC 2020


Hi Aleksey,

>>>  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.

Right. I've added the #if INCLUDE_ALL_GCS and verified with minimal
build that it compiles.

>>> *) 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/

Damn. Should be fixed now.

> 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.

Right. I fixed that all over the place and added the includes too.

>  *) 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-????????

It is actually changed back to do the copy in C2 compiled code, and that
reversion is JDK-8231499... which is part of this backport.

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

Re-ran tests on x86_64. Still looking good.

Roman



More information about the shenandoah-dev mailing list