RFR: 8356125: Interned strings are omitted from AOT cache

[Aleksey Shipilev]

On Mon, 5 May 2025 00:10:38 GMT, Ioi Lam <iklam at openjdk.org> wrote:

> When dumping the interned string table in the AOT cache, we try to include only the strings that are inside ConstantPool::reference_array(). The hope is to limit the size of the AOT cache by omitting interned strings that are not used by objects inside the AOT cache.
> 
> However, we have found two cases when the above scheme doesn't work. Please see the new test cases.
> 
> The fix is to always include all interned strings managed by stringTable.cpp. We might try to omit the truly unused strings in a separate RFE.

I think there is a generic problem in storing the entirety of `StringTable`: we may end up storing _a lot_ of excess `String`-s, and thus blow up the CDS archive size. I've known libraries that (ab)used `String.intern` for string deduplication, storing millions of `String`-s there. 

Do these bugs only reasonably affect potentially interned `String`-s that are reachable from `static` fields of initialized classes? Can we somehow only store those?

src/hotspot/share/cds/heapShared.cpp line 609:

> 607: 
> 608: void HeapShared::archive_strings() {
> 609:   oop shared_strings_array = StringTable::init_shared_strings_array();

I see the old comment here that we always succeed, because `StringTable::init_shared_table` does not create any large arrays. Is this still true? I see this in `StringTable::allocate_shared_strings_array`:


    if (ArchiveHeapWriter::is_too_large_to_archive(secondary_array_size)) {
      // This can only happen if you have an extremely large number of classes that
      // refer to more than 16384 * 16384 = 26M interned strings! Not a practical concern
      // but bail out for safety.
      log_error(cds)("Too many strings to be archived: %zu", _items_count);
      MetaspaceShared::unrecoverable_writing_error();
    }


If we archive the _entirety_ of `StringTable` now, then it is plausible we could archive > 26M Strings now? Maybe write a stress test to see that we are properly failing out of that? Can be (should be?) a follow-up.

test/hotspot/jtreg/runtime/cds/appcds/aotClassLinking/NonFinalStaticWithInitVal.java line 64:

> 62: 
> 63: class MyTestApp {
> 64:     volatile static int x = 0;

Seems unused?

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

PR Review: https://git.openjdk.org/jdk/pull/25026#pullrequestreview-2814332092
PR Review Comment: https://git.openjdk.org/jdk/pull/25026#discussion_r2075923092
PR Review Comment: https://git.openjdk.org/jdk/pull/25026#discussion_r2073165457