On Mon, 28 Oct 2024 23:15:54 GMT, Vladimir Ivanov <vliva...@openjdk.org> wrote:
> I have mixed feelings about proposed fix. It does look like an improvement > over current behavior, but does it really address the root cause of the > problem? I think it does. It is important to separate two problems here: one of JLS conformance, and another of quality-of-implementation. First problem is the behavior for boxed Integers in [-128; 127] range. The caching behavior is required by JLS, and therefore the code can rely on `==` comparisons in this range. If CDS archive and runtime cache has different ideas on the identity of boxed Integers in that range, this is a JLS violation. If CDS has the archived `IntegerCache`, we have to pessimistically assume archived heap objects carry some `Integer` boxes from that cache, so we are now committed to use the boxes from [-128; 127] range from that archive's `IntegerCache` to match them. This is not negotiable anymore: runtime _must_ use the same instances. I believe this is what breaks in [JDK-8342642](https://bugs.openjdk.org/browse/JDK-8342642) otherwise. This is what this PR fixes. Note that counter-example of setting the the Integer cache below 127 is not valid: it would break JLS 5.1.7, and it should be impossible to get into that situation. For reference, the code here sanitizes the input: https://github.com/openjdk/jdk/blob/6332e258f91789cf50d07a6929f32ff3aaef1a92/src/java.base/share/classes/java/lang/Integer.java#L946 Second problem is the behavior for boxed Integers outside [-128; 127] range. JLS 5.1.7 has no provisions about the identity of these boxes: neither a guarantee they would be cached, nor a guarantee that they would _not_ be cached. JLS 5.1.7 note explicitly calls that out: "For other values, the rule disallows any assumptions about the identity of the boxed values on the programmer's part. This allows (but does not require) sharing of some or all of these references.". CDS involvement in this second problem is tangential. If the code at dump time relied on identity of Integer boxes outside of [-128; 127] range, and the identity-sensing code gets broken when integer cache is smaller at runtime, well, too bad: that code was never guaranteed to work reliably to begin with. We only make a quality-of-implementation / simplicity concession that if we are able to load boxed instances >127 from the archive, we will do so. So if we dumped with IntegerCache of high X (X > 127) and load archive into runtime with lower high Y (Y > 127; Y < X), the identity of boxes in entire [-128; Y] range would still be consistent with the archived side, even if JLS allows us not to care about (127; Y] part. ------------- PR Comment: https://git.openjdk.org/jdk/pull/21737#issuecomment-2444012161
