RFR: 8341243: Use ArraySupport.SOFT_MAX_ARRAY_LENGTH for max array size in java.base

[Stuart Marks]

On Mon, 30 Sep 2024 15:14:31 GMT, Eirik Bjørsnøs <eirbjo at 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.

As this stands (modulo my other comments) this change is mostly OK. Using the SOFT_MAX value within java.base is fine. Using SOFT_MAX within java.base-related tests is a little suspicious, because it requires the addition of directives that export an internal package from java.base ... but these tests are generally expected to be closely coupled with java.base internals, so they're probably ok too.

I agree with the decision not to expand usage of SOFT_MAX further, as it's really just a guess at a VM implementation limit that's not defined anywhere. (There have been requests to make this public; see [JDK-8246725](https://bugs.openjdk.org/browse/JDK-8246725), which I'm opposed to.) The *real* fix here is to fix the JVM so that it can allocate arrays of Integer.MAX_VALUE. From time to time I hear rumors that this might happen at some point. Meanwhile, let's not expose something with ill-defined semantics like SOFT_MAX beyond java.base.

As an aside, for your own edification, you might want to verify that compile-time constants are indeed inlined, by inspecting the before-and-after bytecode. This is well documented; see JLS 13.1, item 3 of the first numbered list in that section. I'm sure this is well tested, so there's no need for a test for this.

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

PR Comment: https://git.openjdk.org/jdk/pull/21268#issuecomment-2384736938