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

[Calvin Cheung]

On Wed, 26 Jul 2023 01:32:06 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/dynamicArchive.cpp line 193:
> 
>> 191:       }
>> 192:       if (ak != nullptr && (ak->dimension() > 1)) {
>> 193:         // this is the lowest dimension that's not in the static archive
> 
> The `ak->dimension() > 1` should either be removed, or changed into an assert.

I changed it to an assert.

> src/hotspot/share/oops/objArrayKlass.cpp line 323:
> 
>> 321:     DynamicArchive::log_array_class_load(THREAD, this);
>> 322:   }
>> 323: #endif
> 
> The `ObjArrayKlass::array_klass(int n, TRAPS)` function can be called multiple times, so you'd have duplicated output of "class, load, cds" for the same array class.
> 
> I think it's better to move the logging code from `DynamicArchive::log_array_class_load()` into a function like `ArrayClass::log_array_class_load()`. It can be called from:
> 
> - `ArrayKlass(Symbol* name, KlassKind kind)` constructor
> - `ArrayKlass::restore_unshareable_info()`
> 
> That way we can catch the actual loading event, and also print out whether the class is in static CDS archive, dynamic CDS archive, or not in CDS.
> 
> Instead of `(class, load, cds)`, maybe we can use a new tag like `(class,load,array)`?

I've made the above change.
For array classes not loaded from CDS archive, there won't be `source` info.
Example output:

[0.048s][debug][class,load,cds] [[Ljava.lang.Object; source: shared objects file
[0.054s][debug][class,load,cds] [[[J source: shared objects file (top)
[0.065s][debug][class,load,cds] [Ljava.net.URL;

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

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