On Mon, 28 Oct 2024 22:54:34 GMT, Vladimir Ivanov <vliva...@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 129:
> 
>> 127:                 archivedCache = c;
>> 128:             } else if (archivedCache.length != size) {
>> 129:                 throw new IllegalStateException("CDS archive is broken: 
>> ByteCache mismatch");
> 
> Does `InternalError` better fit the job here?

Right, let's do `InternalError`.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21737#discussion_r1820745317

Reply via email to