RFR(L) 8244778 Archive full module graph in CDS

[Coleen Phillimore]

This is sort of massive, so I'll only make a couple initial comments.

http://cr.openjdk.java.net/~iklam/jdk16/8244778-archive-full-module-graph.v01/src/hotspot/share/classfile/classLoaderData.cpp.udiff.html

All this new code in ClassLoaderData should go somewhere else, like 
maybe classLoaderDataShared.hpp/cpp ?  I don't see how it needs to be 
owned by CLD, except maybe the serialized function.

+ if (loader_data) {


There are many checks that should be != NULL here.

http://cr.openjdk.java.net/~iklam/jdk16/8244778-archive-full-module-graph.v01/src/hotspot/share/classfile/javaClasses.cpp.udiff.html

I don't see why compute_offset/s() has been exported outside 
javaClasses.cpp.

http://cr.openjdk.java.net/~iklam/jdk16/8244778-archive-full-module-graph.v01/src/hotspot/share/classfile/moduleEntry.cpp.udiff.html
http://cr.openjdk.java.net/~iklam/jdk16/8244778-archive-full-module-graph.v01/src/hotspot/share/classfile/packageEntry.cpp.udiff.html

These look like the same code to me.  Maybe they can be generalized and 
put together in a moduleEntryShared.cpp/hpp ?

http://cr.openjdk.java.net/~iklam/jdk16/8244778-archive-full-module-graph.v01/src/hotspot/share/classfile/systemDictionary.cpp.udiff.html

Why do you register the loaders here?

I don't know enough about the Java code to review that.

I assume that this going to be rebased on other changes, so this is only 
an incomplete review.

Coleen


On 7/22/20 3:36 PM, Ioi Lam wrote:
> https://bugs.openjdk.java.net/browse/JDK-8244778
> http://cr.openjdk.java.net/~iklam/jdk16/8244778-archive-full-module-graph.v01/ 
>
>
> Please review this patch that stores the full module graph in the CDS
> archive heap. This reduces the initialization time of the basic JVM by
> about 22%:
>
> $ perf stat -r 100 bin/java -version
> before: 98,219,329 instructions 0.03971 secs elapsed (+- 0.44%)
> after:  55,835,815 instructions 0.03109 secs elapsed (+- 0.65%)
>
> [1] Start with ModuleBootstrap.java. The current implementation is
>     quite restrictive: the archived module graph is used only when no
>     module options are specified.
>
>     See ModuleBootstrap.mayUseArchivedBootLayer().
>
>     We can probably support options such as main module and module path
>     in a future RFE.
>
> [2] In the current JDK implementation, there is no single object
>     that represents "the module graph". Most of the information
>     is stored in the archive bootLayer object, but a few additional
>     restoration operations need to be performed:
>
>     + See ModuleBootstrap.getArchivedBootLayer()
>     + Some static fields need to be archived/restored in
>       Module.java, BuiltinClassLoader.java, ClassLoaders.java
>       and BootLoader.java
>
> [3] I ran into a complication with two loader instances of
>     PlatformClassLoader and AppClassLoader. They are stored in
>     multiple tables inside the module graph (e.g.,
>     BuiltinClassLoader$LoadedModule) so I cannot easily recreate
>     them at runtime.
>
>     However, these two loaders contain information specific to the
>     dump time VM lifecycle (such as the classes that were loaded
>     during CDS dumping) that need to be scrubbed. I couldn't find an
>     elegant way of doing this, so I added a private "resetArchivedStates"
>     method to a few classes. They are called inside
>     HeapShared::reset_archived_object_states().
>
> [4] Related native data structures (PackageEntry and ModuleEntry)
>     are also archived. Start with classLoaderData.cpp
>
> Passes mach5 tiers 1-4. I will test with additional tiers.
>
> Thanks
> - Ioi