RFR(L) 8244778 Archive full module graph in CDS
Coleen Phillimore
coleen.phillimore at oracle.com
Tue Aug 4 14:49:54 UTC 2020
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
More information about the hotspot-runtime-dev
mailing list