On Mon, 28 Oct 2024 10:16:40 GMT, Aleksey Shipilev <sh...@openjdk.org> wrote:
> This is forked from > [JDK-8342642](https://bugs.openjdk.org/browse/JDK-8342642) and filed as a > general issue for archived boxed Integer cache when it's recreated at > runtime. In short, current code drops the entire primitive cache when the CDS > archived version of the cache is too short. This poses a problem with code > that uses CDS archived cache instances, since the boxed equality would break > when comparing the CDS-archived value and the IntegerCache value recreated at > runtime. > > Ioi suggested a fix here: > https://github.com/openjdk/jdk/pull/21672#issuecomment-2434359711. I > separately arrived to the same idea. This PR implements it. `IntegerCache` > gets the special treatment, because it is the only cache that can be tuned. > Other caches just prevent the use of bad archived cache (which I think should > realistically never happen) without re-creating it. > > Unfortunately, I was unable to create a standalone CDS test for this: I > somehow need to be able to put my own archived `Integer` instance into the > archive, and I don't clearly see how. I tested this patch with original > reproducer from [JDK-8342642](https://bugs.openjdk.org/browse/JDK-8342642) -- > and it starts to pass. I don't think we would be able to convert that > reproducer into a unit test, since > [JDK-8342642](https://bugs.openjdk.org/browse/JDK-8342642) itself would make > the reproducer ineffective. > > Additional testing: > - [x] macos-aarch64-server-fastdebug, > [JDK-8342642](https://bugs.openjdk.org/browse/JDK-8342642) reproducer now > passes > - [x] linux-aarch64-server-fastdebug, `all` src/java.base/share/classes/java/lang/Byte.java line 128: > 126: } > 127: archivedCache = c; > 128: } else if (archivedCache.length != size) { The `else` case for the non-int boxed type should never happen. Add `assert archivedCache.length == size;` in the if case instead? src/java.base/share/classes/java/lang/Integer.java line 964: > 962: Integer[] c = new Integer[size]; > 963: int j = low; > 964: // Use all cached values from the archive to avoid > breaking Alternatively, you could use a separate array to store **only** the runtime created cached instances, without copying the references from the archived cache. Both the archived cache and runtime added cache are used. That has slightly less memory and efficiency overhead, but almost unnoticeable. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/21737#discussion_r1819852002 PR Review Comment: https://git.openjdk.org/jdk/pull/21737#discussion_r1819857977
