RFR: 8343019: Primitive caches must use boxed instances from the archive [v4]

Ioi Lam iklam at openjdk.org
Wed Oct 30 18:12:10 UTC 2024


On Wed, 30 Oct 2024 10:07:22 GMT, Aleksey Shipilev <shade at 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.
>> 
>> I tested this patch with original reproducer from [JDK-8342642](https://bugs.openjdk.org/browse/JDK-8342642) -- and it starts to pass.
>> 
>> Additional testing:
>>  - [x] macos-aarch64-server-fastdebug, [JDK-8342642](https://bugs.openjdk.org/browse/JDK-8342642) reproducer now passes
>>  - [x] macos-aarch64-server-fastdebug, new regression test fails without the fix, passes with it
>>  - [x] linux-aarch64-server-fastdebug, `all`
>
> Aleksey Shipilev has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Change InternalErrors to asserts

LGTM. One small nit with variable names.

src/java.base/share/classes/java/lang/Integer.java line 966:

> 964:                 // Otherwise, the identity checks between archived Integers and
> 965:                 // runtime-cached Integers would fail.
> 966:                 int archivedIdx = (archivedCache == null) ? 0 : archivedCache.length;

I think the name of `archiveIdx` isn't clear. How about `archivedSize`?

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

Marked as reviewed by iklam (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/21737#pullrequestreview-2405844283
PR Review Comment: https://git.openjdk.org/jdk/pull/21737#discussion_r1823153750


More information about the core-libs-dev mailing list