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

[Ioi Lam]

On Fri, 22 Jan 2021 23:59:34 GMT, Calvin Cheung <ccheung 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%

Changes requested by iklam (Reviewer).

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

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

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