RFR: 8309240: Array classes should be stored in dynamic CDS archive [v2]

[Ioi Lam]

On Wed, 26 Jul 2023 00:05:56 GMT, Calvin Cheung <ccheung at openjdk.org> wrote:

>> Please review this RFE for adding array classes in the dynamic CDS archive.
>> 
>> During dynamic dump, only the array classes with the `element_klass` already in the static archive will be stored in the dynamic archive. See `ArchiveBuilder::gather_array_klasses()`. It is because the array classes which are in the same archive (either static or dynamic) are already "connected" together. Higher dimension primitive array classes are also stored in the dynamic archive. Note that one-dimensional primitive array classes are store in the static archive.
>> 
>> During runtime, the `DynamicArchive::setup_array_klasses()` is for setting up the array classes. If only the `bottom_klass` is in the static archive, the function will point the `array_klasses` to the stored array classes in the dynamic archive. If a lower dimension array class is already in the static archive, its `higher_dimension` will be set to refer to the stored array classes in the dynamic archive.
>> 
>> Some logging is added to `ObjArrayKlass::array_klass(int n, TRAPS)` to indicate an array class is being loaded from the archive.
>> 
>> Passed tiers 1 - 4 testing.
>
> Calvin Cheung has updated the pull request incrementally with one additional commit since the last revision:
> 
>   simplify the fix

src/hotspot/share/cds/archiveBuilder.cpp line 817:

> 815:           assert(ArrayKlass::cast(elem)->higher_dimension() == oak, "must be");
> 816:         } else {
> 817:           assert(InstanceKlass::cast(elem)->array_klasses() == oak, "must be");

These two asserts don't do what the comment says. They check that the element klass of `oak` points back to `oak`, but that's not a property that's required by this PR.

To assert what the comment says, it would be:

     ObjArrayKlass* oak = ObjArrayKlass::cast(klasses()->at(i));
+    assert(!MetaspaceShared::is_shared_static(oak), "we should not gather klasses that are already in the static archive");

src/hotspot/share/cds/dynamicArchive.cpp line 377:

> 375: 
> 376: GrowableArray<ArrayKlass*>* DynamicArchive::_array_klasses = nullptr;
> 377: Array<ArrayKlass*>* DynamicArchive::_dynamic_archive_array_klasses = nullptr;

These two arrays can be declared to contain `<ObjArrayKlass*>`, as that's the only type of object you store in there. That way, you can avoid type casting when reading elements from `_dynamic_archive_array_klasses` at runtime.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/14959#discussion_r1274288965
PR Review Comment: https://git.openjdk.org/jdk/pull/14959#discussion_r1274286248