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%

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

> 1057:   PackageEntry* pkg_entry = ik->package();
> 1058:   if (MetaspaceShared::use_full_module_graph() && ik->is_shared() &&
> 1059:        ((pkg_entry == NULL) || MetaspaceShared::is_in_shared_metaspace(pkg_entry))) {

I think the `(pkg_entry == NULL)` check will be incorrect if the class is loaded by a custom loader, or is in an unnamed module.

Also, I wonder if you added the check `MetaspaceShared::is_in_shared_metaspace(pkg_entry)` because of the missing `ArchiveUtils::mark_pointer()` call.

How about changing to

if (MetaspaceShared::use_full_module_graph() && ik->is_shared() && pkg_entry != NULL) {
  assert(MetaspaceShared::is_in_shared_metaspace(pkg_entry), "must be");
  // also assert not in unnamed module, and not unregistered loader ...
  return pkg_entry;
}

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

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