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