On Mon, 30 Sep 2024 15:14:31 GMT, Eirik Bjørsnøs <eir...@openjdk.org> wrote:
> Please review this cleanup PR which updates code and tests in `java.base` to > consistently use `jdk.internal.util.ArraySupport.SOFT_MAX_ARRAY_LENGTH` when > referring to the JVM's maximum array size implementation limit. Currently, > instances of `Integer.MAX_VALUE - 8` are found across the code base, with > varying degrees of documentation. It would be good to consolidate on a single > source of truth, with proper documentation. > > This PR is a follow-up to #20905 where the same change was requested in > `java.util.zip`. > > My understanding is that javac will fold this constant value into the byte > code of the compiled use sites, as such this change should not affect class > loading or cause startup issues. > > Instances selected for this PR were found searching for "Integer.MAX_VALUE - > 8". The PR replaces these with `ArraySupport.SOFT_MAX_ARRAY_LENGTH`, while > trimming or amending some code comments where appropriate. > (`SOFT_MAX_ARRAY_LENGTH` already has a good explainer which does not need > repetition at each use site). > > I also searched for instances of `Integer.MAX_VALUE - 1` and > `Integer.MAX_VALUE - 2`, no convincing candidates were found. > > Instances outside `java.base` were deliberately left out to limit the scope > and review cost of this PR. > > Tests updated to use `SOFT_MAX_ARRAY_LENGTH` are updated with the jtreg tag > `@modules java.base/jdk.internal.util`. > > Testing: No new tests are added in this PR, the `noreg-cleanup` label is > added to the JBS. The five affected tests have been run manually. GHA tests > run green. test/jdk/java/lang/StringBuffer/HugeCapacity.java line 51: > 49: // Maximum array length supported by the JVM implementation > 50: int maxArrayLength = ArraysSupport.SOFT_MAX_ARRAY_LENGTH; > 51: String str = "Z".repeat(maxArrayLength); Same comment as in the corresponding code in the StringBuilder test. test/jdk/java/lang/StringBuilder/HugeCapacity.java line 83: > 81: // Maximum array length supported by the JVM implementation > 82: int maxArrayLength = ArraysSupport.SOFT_MAX_ARRAY_LENGTH; > 83: String str = "Z".repeat(maxArrayLength); The comment is a bit misleading. I'm not sure any comment is warranted, as you had observed, there's a big description next to SOFT_MAX_ARRAY_LENGTH. I'm also not sure a new local variable maxArrayLength adds anything. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/21268#discussion_r1782068899 PR Review Comment: https://git.openjdk.org/jdk/pull/21268#discussion_r1782068601
