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

[Calvin Cheung]

On Wed, 26 Jul 2023 02:01:44 GMT, Ioi Lam <iklam at openjdk.org> wrote:

>> 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");

Fixed.

> 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.

I changed the array to `<ObjArrayKlass*>`, it reduces couple of type casting.

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

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