RFR: 8357525: Default CDS archive becomes non-deterministic after JDK-8305895 [v2]

[Aleksey Shipilev]

On Tue, 27 May 2025 18:12:32 GMT, Ioi Lam <iklam at openjdk.org> wrote:

>> Since [JDK-8305895](https://bugs.openjdk.org/browse/JDK-8305895), interfaces and abstract classes are allocated outside of the class space. See [here for the triggering condition](https://github.com/openjdk/jdk/blame/4d7068923cd87fbfc2edee25406521b11580d153/src/hotspot/share/classfile/classFileParser.cpp#L5835-L5842)
>> 
>> Theoretically such InstanceKlasses can have arbitrary addresses, so the  "tie breaker test" of `(uintptr_t(existing) < uintptr_t(klass))` in [Klass::hash_insert()](https://github.com/openjdk/jdk/blob/4d7068923cd87fbfc2edee25406521b11580d153/src/hotspot/share/oops/klass.cpp#L458) no longer produces a deterministic result across two JVM runs. As a result, we have seen several occurrence of non-deterministic CDS archives in our test pipeline for a short period after  [JDK-8305895](https://bugs.openjdk.org/browse/JDK-8305895) for pushed.
>> 
>> Thereafter, for some unknown reason, the problem disappears. However, we could just be lucky due to allocation policy changes in metaspace. The underlying cause still exists.
>> 
>> I wrote a proof of concept that shows the problem. See https://github.com/openjdk/jdk/commit/33185705f85986e1ee1e529005898e834cc8a88f
>> 
>> 
>> (Without fix)
>> 
>> $ java -Xshare:dump -XX:+UseNewCode -XX:SharedArchiveFile=f1.jsa
>> $ java -Xshare:dump -XX:+UseNewCode2 -XX:SharedArchiveFile=f2.jsa
>> $ cksum f1.jsa f2.jsa
>>   2834014305 16105472 f1.jsa
>>   4126337207 16105472 f2.jsa
>> 
>> (With the fix in this PR)
>> 
>> $ java -Xshare:dump -XX:+UseNewCode -XX:+UseNewCode3 -XX:SharedArchiveFile=f1.jsa
>> $ java -Xshare:dump -XX:+UseNewCode2 -XX:+UseNewCode3 -XX:SharedArchiveFile=f2.jsa
>> $ cksum f1.jsa f2.jsa
>>   3637571299 16105472 f1.jsa
>>   3637571299 16105472 f2.jsa
>> 
>> 
>> With the fix, we re-hash the copy of the `secondary_supers` in the CDS archive. When InstanceKlasses are copied into the CDS archive, they are sorted by their names. This makes the `(uintptr_t(existing) < uintptr_t(klass))`  comparison produce deterministic results.
>> 
>> Unfortunately it's not possible to write a jtreg test for this problem.
>> 
>> Note: the "tie breaker test" was introduced in [JDK-8180450](https://bugs.openjdk.org/browse/JDK-8180450). It adds an assumption that the address order of classes in the CDS archive buffer is the same as the classes in the class space, which can become invalid if the class space allocation algorithm changes.
>
> Ioi Lam has updated the pull request incrementally with one additional commit since the last revision:
> 
>   @shipilev comments -- fixed typo

I don't see the bug synopsis update, but PR looks good.

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

Marked as reviewed by shade (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/25373#pullrequestreview-2872226362