On Wed, 30 Oct 2024 10:07:22 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.
>> 
>> 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

Reply via email to