RFR: 8249262: Initialize InstanceKlass::_package_entry during CDS dump time

[Ioi Lam]

On Mon, 25 Jan 2021 17:03:55 GMT, Ioi Lam <iklam at openjdk.org> wrote:

>> With JDK-8244778, the `PackageEntry` of all archived classes (for the 3 built-in class loaders) are all created during CDS dump time. We can remember this in `InstanceKlass::_package_entry` during dump time to avoid the hashtable lookup at runtime in `InstanceKlass::set_package()` and `SystemDictionaryShared::get_package_entry_from_class_name()`.
>> 
>> Passed tiers 1 - 4 testing.
>> 
>> Just a very small perf improvement with HelloWorld:
>> instr delta =        -9514    -0.0145%
>> time  delta =       -0.272 ms -0.6916%
>
> src/hotspot/share/oops/instanceKlass.cpp line 2558:
> 
>> 2556:     _package_entry = NULL;
>> 2557:   }
>> 2558: #endif
> 
> I think the `#if` part is not needed. `MetaspaceShared::use_full_module_graph()` returns false if `INCLUDE_CDS_JAVA_HEAP` is false.
> 
> Instead of `ArchiveBuilder::update_package_entry()`, maybe we can set `_package_entry` here?
> 
> if (!MetaspaceShared::use_full_module_graph()) {
>   _package_entry = NULL;
> } else if (DynamicDumpSharedSpaces) {
>   if (!Metaspace::is_in_shared_metaspace(_package_entry)) {
>     _package_entry = NULL;  
>   }
> } else {
>   _package_entry =  PackageEntry::get_archived_entry(entry);
> }
> ArchiveUtils::mark_pointer((address**)&_package_entry);
> 
> The `mark_pointer` call is necessary if relocation happens during dumping.
> 
> For classes loaded by custom loaders, we don't archive their modules so get_archived_entry() should return a NULL package. I think for sanity, we should add an assert like:
> 
> if (ik->is_shared_unregistered_class()) {
>   assert(_package_entry == NULL, "package entry for unregistered classes are not archived");
> }

Also need to assert `_package_entry == NULL` if the class is in an unnamed module.

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

PR: https://git.openjdk.java.net/jdk/pull/2206