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

[Ashutosh Mehra]

On Mon, 31 Jul 2023 22:12:23 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 with a new target base due to a merge or a rebase. The pull request now contains five commits:
> 
>  - Merge branch 'master' into 8309240-array-classes-dynamic-cds
>  - comments from Ioi and David
>  - @iklam comments
>  - simplify the fix
>  - 8309240: Array classes should be stored in dynamic CDS archive

src/hotspot/share/classfile/systemDictionaryShared.cpp line 193:

> 191:     assert(!k->is_shared(), "sanity");
> 192:   } else {
> 193:     assert(DynamicDumpSharedSpaces, "sanity");

I am trying to understand this block of code. Does it imply `k` can be a shared class when `DynamicDumpSharedSpaces` is set?  When can this happen? 
If `k` can be a shared class here, then the assert in the next call to `DumpTimeClassInfo::get_info` for `k` to be a non-shared class will be triggered. So there seem to be some inconsistency in the asserts, unless I am reading it wrong.

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

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