RFR: 8312749: Generational ZGC: Tests crash with assert(index == 0 || is_power_of_2(index))

[Tobias Hartmann]

On Tue, 15 Aug 2023 12:43:56 GMT, Roberto Castañeda Lozano <rcastanedalo at openjdk.org> wrote:

> This changeset ensures that the array copy stub underlying the intrinsic implementation of `Object.clone` only copies its (double-word aligned) payload, excluding the remaining object alignment padding words, when a non-default `ObjectAlignmentInBytes` value is used. This prevents the specialized ZGC stubs for `Object[]` array copy from processing undefined object alignment padding words as valid object pointers. For further details about the specific failure, see [initial analysis](https://bugs.openjdk.org/browse/JDK-8312749?focusedCommentId=14600658&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-14600658) by Erik Österlund and Stefan Karlsson and comments in the regression test included in this changeset.
> 
> As a side-benefit, the changeset simplifies the array size computation logic in `GraphKit::new_array()` by decoupling computation of header size and alignment padding size.
> 
> #### Testing
> 
> ##### Functionality
> 
> - tier1-3 (windows-x64, linux-x64, linux-aarch64, macosx-x64, and macosx-aarch64)
> - tier4-5 (windows-x64, linux-x64, linux-aarch64, macosx-x64, and macosx-aarch64; ZGC-specific tests only)
> - tier6-9 (linux-x64; ZGC-specific tests only)
> - tier1-3, and a few custom examples, applying [JDK-8139457](https://github.com/openjdk/jdk/pull/11044) (under review) on top of this changeset
> 
> ##### Performance
> 
> Tested performance on the following set of OpenJDK micro-benchmarks, on linux-x64 (for both G1 and ZGC, using different ObjectAlignmentInBytes values):
> 
> - `openjdk.bench.java.lang.ArrayClone.byteClone`
> - `openjdk.bench.java.lang.ArrayClone.intClone`
> - `openjdk.bench.java.lang.ArrayFiddle.simple_clone`
> - `openjdk.bench.java.lang.Clone.cloneLarge`
> - `openjdk.bench.java.lang.Clone.cloneThreeDifferent`
> 
> No significant regression was observed.

Looks good to me!

src/hotspot/share/gc/shared/c2/barrierSetC2.cpp line 686:

> 684:   }
> 685:   payload_size = kit->gvn().transform(new URShiftXNode(payload_size, kit->intcon(LogBytesPerLong)));
> 686:   ArrayCopyNode* ac = ArrayCopyNode::make(kit, false, src_base, offset,  dst_base, offset, payload_size, true, false);

Suggestion:

  ArrayCopyNode* ac = ArrayCopyNode::make(kit, false, src_base, offset, dst_base, offset, payload_size, true, false);

-------------

Marked as reviewed by thartmann (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/15288#pullrequestreview-1586603759
PR Review Comment: https://git.openjdk.org/jdk/pull/15288#discussion_r1299866583