RFR(L) 8244778 Archive full module graph in CDS
Ioi Lam
ioi.lam at oracle.com
Wed Aug 12 22:11:49 UTC 2020
Hi Coleen,
Thanks for the comments. I have an updated webrev
http://cr.openjdk.java.net/~iklam/jdk16/8244778-archive-full-module-graph.v02/
http://cr.openjdk.java.net/~iklam/jdk16/8244778-archive-full-module-graph.v02.delta/
On 8/4/20 7:49 AM, Coleen Phillimore wrote:
> 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.
>
>
Fixed
> + if (loader_data) {
>
>
> There are many checks that should be != NULL here.
>
Fixed and added comments
> 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.
>
It was left-over code that shouldn't be there. I removed it.
> 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 ?
>
They look similar but also do different things. Any suggestion for how
to generalize them?
> 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?
>
It's to force the creation of the ClassLoaderData if a loader oop
doesn't have one yet. Also, in the new version, I moved the code to
Modules::define_archived_modules().
Thanks
- Ioi
> 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