RFR: 8301106 allow cds interned strings to be moved by gc [v2]

[David Holmes]

On Sat, 18 Feb 2023 04:49:29 GMT, Ioi Lam <iklam at openjdk.org> wrote:

>> **Background:**
>> 
>> Currently, the archived java strings are mapped in the G1 "[closed archive](https://github.com/openjdk/jdk/blob/574b48c6925ebfb31345fc46c7d23aa4153f99b0/src/hotspot/share/gc/g1/heapRegionType.hpp#L80-L92)" region. This essentially pins all the strings in memory.
>> 
>> As a prerequisite for  ([JDK-8296263](https://bugs.openjdk.org/browse/JDK-8296263)), this PR removes the requirement of pinning the archived strings. This will allow the CDS archive heap to be mapped in garbage collectors that do not support object pinning.
>> 
>> **Code changes:**
>> 
>> - The archived strings are referenced through an objArray (`_shared_strings_array`) to keep them alive. As a result, it's no longer necessary to pin them.
>> - Since it's possible for the GC to move these strings, the `_shared_table` in stringTable.cpp is modified to store a 32-bit index for each archived string. This index is used to retrieve the archived string from `_shared_strings_array` at runtime.
>> 
>> Note that CDS has a limit on the size of archived objArrays. When there's a large number of strings, we use a two-level table. See the comments around  `_shared_strings_array` in the header file.
>> 
>> **Testing**
>> 
>> Tiers 1 - 4
>
> Ioi Lam has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - renamed obsolete functions to ArchiveHeapLoader::is_in_use()
>  - fixed typos in comment

Still working through this. A few comments.

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

> 424:   oop shared_strings_array = StringTable::init_shared_table(_dumped_interned_strings);
> 425:   bool success = archive_reachable_objects_from(1, _default_subgraph_info, shared_strings_array, /*is_closed_archive=*/ false);
> 426:   guarantee(success, "shared strings array must point to only archivable objects");

What could cause this to fail? Do we need a more graceful bailout in release builds?

src/hotspot/share/classfile/stringTable.cpp line 76:

> 74: 
> 75: #if INCLUDE_CDS_JAVA_HEAP
> 76: bool StringTable::_two_dimensional_shared_strings_array = false;

Suggestion: `_is_two_dimensional`

src/hotspot/share/classfile/stringTable.cpp line 788:

> 786:     objArrayOop array = oopFactory::new_objArray(vmClasses::Object_klass(), total, CHECK);
> 787:     _shared_strings_array = OopHandle(Universe::vm_global(), array);
> 788:     _two_dimensional_shared_strings_array = false;

It was initialized to false.

src/hotspot/share/classfile/stringTable.hpp line 128:

> 126:   //     ArchiveHeapWriter::is_too_large_to_archive(). In this case, the index is splited into two
> 127:   //     parts using the follow bit-opts. Each the shared string is stored as
> 128:   //     _shared_strings_array[upper][lower]

upper/lower don't match use of primary/secondary. I'm still unclear how the 32-bit index is split in the two dimensional case.

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

PR: https://git.openjdk.org/jdk/pull/12607