RFR: 8315082: [REDO] Generational ZGC: Tests crash with assert(index == 0 || is_power_of_2(index))
Roberto Castañeda Lozano
rcastanedalo at openjdk.org
Mon Sep 11 07:08:05 UTC 2023
This changeset (REDO of [JDK-8312749](https://bugs.openjdk.org/browse/JDK-8312749)) 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 of [JDK-8312749](https://bugs.openjdk.org/browse/JDK-8312749) by Erik Österlund and Stefan Karlsson and comments in `test/hotspot/jtreg/compiler/gcbarriers/TestArrayCopyWithLargeObjectAlignment.java`.
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.
#### Additional changes compared to [JDK-8312749](https://bugs.openjdk.org/browse/JDK-8312749)
This changeset proposes the exact same solution as [JDK-8312749](https://bugs.openjdk.org/browse/JDK-8312749), that is, identical changes to `barrierSetC2.cpp`, `graphKit.cpp`, `library_call.cpp`, and `TestArrayCopyWithLargeObjectAlignment.java`. On top of that, it relaxes an assertion in the idealization of `ArrayCopy` nodes violated by [JDK-8312749](https://bugs.openjdk.org/browse/JDK-8312749) and reported in [JDK-8315029](https://bugs.openjdk.org/browse/JDK-8315029) (new changes in `arraycopynode.cpp`, new regression test `TestCloneArrayWithDifferentLengthConstness.java`). The original, stricter assertion checks that, while idealizing an ArrayCopy node, the "constness" of the array copy's word-length (whether it is known by C2 to be constant or not) is equivalent to that of the array copy's element-length. For cases in which the element-length is within a small, fixed range (e.g. for an `int` array of length `3..4`) so that all element-length values lead to the same number of word
s (`2`), the assertion used to hold before this changeset only because of weak type propagation in `AndL` (preventing the constant word-length to be discovered), see the left graph below:
With the proposed changes, the array copy word-length is computed in a more straightforward way that enables C2 to infer the precise number of words in the same scenario (see the right graph above). To accommodate this optimization, this changeset relaxes the assertion to check only one direction of the implication (if the element-length is constant, so is the word-length). The optimization does not affect the remaining idealization code, since it only uses element-length in the context in which the optimization is applied.
The changeset includes an additional regression test (`TestCloneArrayWithDifferentLengthConstness.java`) that exercises different constant/variable combinations of element-length and word-length.
#### Testing
##### Functionality
- tier1-5 (windows-x64, linux-x64, linux-aarch64, macosx-x64, and macosx-aarch64)
- tier6-7 (linux-x64 only)
- compiler-focused stress testing
- failing tests reported in [JDK-8315029](https://bugs.openjdk.org/browse/JDK-8315029)
- tier1-3, and a few custom examples, applying [JDK-8139457](https://github.com/openjdk/jdk/pull/11044) 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.
-------------
Commit messages:
- Add regression test that exercises the relaxed assertion
- Relaxed assertion
- Remove extra whitespace
- Remove extra whitespace
- Revert use of UseNewCode
- Revert "TEMPORARY: add additional macro-assembly comments"
- Revert "TEMPORARY: set UseNewCode to true by default"
- Revert "TEMPORARY: print only 'oop_disjoint_arraycopy_uninit' stub code"
- Require GenZGC in the test
- Round up object size at the end of the computation
- ... and 11 more: https://git.openjdk.org/jdk/compare/024133b0...b3109e30
Changes: https://git.openjdk.org/jdk/pull/15589/files
Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=15589&range=00
Issue: https://bugs.openjdk.org/browse/JDK-8315082
Stats: 188 lines in 6 files changed: 157 ins; 10 del; 21 mod
Patch: https://git.openjdk.org/jdk/pull/15589.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/15589/head:pull/15589
PR: https://git.openjdk.org/jdk/pull/15589
More information about the hotspot-gc-dev
mailing list