RFR: 8344904: Interned strings in old classes are not stored in CDS archive [v2]

[Ioi Lam]

On Mon, 25 Nov 2024 01:16:24 GMT, David Holmes <dholmes at openjdk.org> wrote:

> Plus if this is only about the CDS archive can we say that in the title instead of AOT cache? IIUC this is an existing CDS issue and unrelated to any recent AOT/Leyden integrations - right?

After JEP 483, we will gradually switch over to the term of "AOT" to describe all ahead-of-time features (including class metadata processing, bytecode compilation, Java object generation, etc). The term "CDS" will be deemphasized as it's a historical acronym that doesn't accurate describe its purpose anymore, but we will probably still use it when convenient (as the word CDS still appears a lot in the source code).

For this issue, though, since it might be backported, I changed the title back to "CDS" so it will not look out of place in older releases.

> src/hotspot/share/oops/constantPool.cpp line 411:
> 
>> 409:   if (!ik->is_linked()) {
>> 410:     // resolved_references() doesn't exist yet, so we have no resolved CONSTANT_String entries. However,
>> 411:     // Some static final fields may have default values that were initialized when the class was parsed.
> 
> Suggestion:
> 
>     // some static final fields may have default values that were initialized when the class was parsed.

Fixed

> src/hotspot/share/oops/constantPool.cpp line 412:
> 
>> 410:     // resolved_references() doesn't exist yet, so we have no resolved CONSTANT_String entries. However,
>> 411:     // Some static final fields may have default values that were initialized when the class was parsed.
>> 412:     // We need to enter those into the CDS archived strings table.
> 
> Suggestion:
> 
>     // We need to enter those into the CDS archive strings table.

Fixed

> test/hotspot/jtreg/runtime/cds/appcds/sharedStrings/OldClassWithStaticString.jasm line 35:
> 
>> 33: 
>> 34: public super class OldClassWithStaticString
>> 35:     version 49:0
> 
> Is this a jasm file only because we need the version 49? If so please state that somewhere.

Fixed.

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

PR Comment: https://git.openjdk.org/jdk/pull/22340#issuecomment-2499701207
PR Review Comment: https://git.openjdk.org/jdk/pull/22340#discussion_r1857803999
PR Review Comment: https://git.openjdk.org/jdk/pull/22340#discussion_r1857804056
PR Review Comment: https://git.openjdk.org/jdk/pull/22340#discussion_r1857804181