On Tue, 1 Oct 2024 04:01:09 GMT, Stuart Marks <sma...@openjdk.org> wrote:
> 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 was pondering a bit whether to include tests. The use of the constant perhaps aids understanding even more here, because the value might seem "more magic" , being further removed from where actual allocation takes place. Tests are a useful tool for understanding APIs and their implementation, so less magic feels like an improvement. Also, we want to update tests as well if the implementation limit is lifted, this makes that update easier. > 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. Good, I initially included code outside `java.base`, but that quicly got messy. > 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. Thanks for the JLS pointer, good that this is not just a javac implementation detail. Confirmed this using: % javap -c -p -constants java.io.InputStream | grep MAX_BUFFER_SIZE` private static final int MAX_BUFFER_SIZE = 2147483639; % javap -c -p -constants java.io.InputStream | grep ArraySupport ------------- PR Comment: https://git.openjdk.org/jdk/pull/21268#issuecomment-2384860067
