Hi Calvin,
Thanks for the review! Here is the updated webrevs that address the feedbacks
from you and Ioi:
http://cr.openjdk.java.net/~jiangli/8202035/webrev_inc.01/
Full webrev: http://cr.openjdk.java.net/~jiangli/8202035/webrev_full.01/
On Jul 6, 2018, at 9:15 AM, Calvin Cheung <calvin.che...@oracle.com> wrote:
Hi Jiangli,
Thanks for this start-up improvement. The changes look good overall. I've the
following minor comments.
1) make/hotspot/symbols/symbols-unix
134 JVM_InitializeFromArchive
If you want the symbols to be in alphabetical order, the above should be
moved after JVM_InitStackTraceElementArray.
Fixed.
2) metaspaceShared.cpp
1927 oop MetaspaceShared::materialize_archived_object(oop obj) {
1928 if (obj != NULL) {
1929 return G1CollectedHeap::heap()->materialize_archived_object(obj);
1930 }
1931 return NULL;
1932 }
Instead of two return statements, how about replacing lines 1928 - 1931
with the following?
return (obj != NULL) ?
G1CollectedHeap::heap()->materialize_archived_object(obj) : NULL;
The original format probably is slightly easier to read, so I left it
unchanged. Hope that’s okay with you.
3) ArchivedModuleComboTest.java
55 Path moduleDir = Files.createTempDirectory(userDir, "mods");
I don't see anything got placed under the "mods" dir, is it by design?
Yes.
For the "dump with --module-path" cases, there seems to be a missing test case with
"--show-module-resolution" (similar to Test case 2).
When --module-path is specified at dump time, system module graph is not
archived currently. There is no need for additional test case with
--show-module-resolution in this case since all module objects are created as
normal.
4) CheckArchivedModuleApp.java
53 if (expectArchived && wb.isShared(md)) {
54 System.out.println(name + " is archived. Expected.");
55 } else if (!expectArchived && !wb.isShared(md)) {
56 System.out.println(name + " is not archived. Expected.");
57 } else if (expectArchived) {
58 throw new RuntimeException(
59 "FAILED. " + name + " is not archived. Expect
archived.");
60 } else {
61 throw new RuntimeException(
62 "FAILED. " + name + " is archived. Expect not
archived.");
63 }
I'd suggest the following so that the code is easier to understand:
if (expectArchived) {
if (wb.isShared(md)) {
System.out.println(name + " is archived. Expected.");
} else {
throw new RuntimeException(
"FAILED. " + name + " is not archived. Expect archived.");
}
} else {
if (!wb.isShared(md)) {
System.out.println(name + " is not archived. Expected.");
} else {
throw new RuntimeException(
"FAILED. " + name + " is archived. Expect not archived.");
}
}
Reformatted as suggested.
5) ArchivedModuleWithCustomImageTest.java
178 private static void printCommand(String opts[]) {
179 StringBuilder cmdLine = new StringBuilder();
180 for (String cmd : opts)
181 cmdLine.append(cmd).append(' ');
182 System.out.println("Command line: [" + cmdLine.toString() + "]");
183 }
Consider putting the above method in ProcessTools.java so that
ProcessTools.createJavaProcessBuilder() and the above test can call it and
avoiding duplicate code.
A separate follow-up bug to address this is fine.
That sounds good to me. We might need some reformatting for consolidation. I
will file a follow-up RFE.
6) PrintSystemModulesApp.java
I don't think it is being used?
It’s used by ArchivedModuleCompareTest.java. Looks like it was missing from the
earlier webrev. Thanks for catching that. The file is included in the updated
webrev.
Thanks!
Jiangli
thanks,
Calvin
On 6/28/18, 4:15 PM, Jiangli Zhou wrote:
This is a follow-up RFE of JDK-8201650 (Move iteration order randomization of
unmodifiable Set and Map to iterators), which was resolved to allow Set/Map
objects being archived at CDS dump time (thanks Claes and Stuart Marks). In the
current RFE, it archives the set of system ModuleReference and ModuleDescriptor
objects (including their referenced objects) in 'open' archive heap region at
CDS dump time. It allows reusing of the objects and bypassing the process of
creating the system ModuleDescriptors and ModuleReferences at runtime for
startup improvement. My preliminary measurements on linux-x64 showed ~5%
startup improvement when running HelloWorld from -cp using archived module
objects at runtime (without extra tuning).
The library changes in the following webrev are contributed by Alan Bateman.
Thanks Alan and Mandy for discussions and help. Thanks Karen, Lois and Ioi for
discussion and suggestions on initialization ordering.
The majority of the module object archiving code are in heapShared.hpp and
heapShared.cpp. Thanks Coleen for pre-review and Eric Caspole for helping
performance tests.
webrev: http://cr.openjdk.java.net/~jiangli/8202035/webrev.00/
RFE: https://bugs.openjdk.java.net/browse/JDK-8202035?filter=14921
Tested using tier1 - tier6 via mach5 including all new test cases added in the
webrev.
Following are the details of system module archiving, which are duplicated in
above bug report.
---------------------------------------------------------------------------------------------------------------------------
Support archiving system module graph when the initial module is unnamed module
from -cp currently.
Support G1 GC, 64-bit (non-Windows). Requires UseCompressedOops and
UseCompressedClassPointers.
Dump time system module object archiving
=================================
At dump time, the following fields in ArchivedModuleGraph are set to record the
system module information created by ModuleBootstrap for archiving.
private static SystemModules archivedSystemModules;
private static ModuleFinder archivedSystemModuleFinder;
private static String archivedMainModule;
The archiving process starts from a given static field in ArchivedModuleGraph
class instance (java mirror object). The process archives the complete network
of java heap objects that are reachable directly or indirectly from the
starting object by following references.
1. Starts from a given static field within the Class instance (java mirror). If
the static field is a refererence field and points to a non-null java object,
proceed to the next step. The static field and it's value is recorded and
stored outside the archived mirror.
2. Archives the referenced java object. If an archived copy of the current
object already exists, updates the pointer in the archived copy of the
referencing object to point to the current archived object. Otherwise, proceed
to the next step.
3. Follows all references within the current java object and recursively
archive the sub-graph of objects starting from each reference encountered
within the object.
4. Updates the pointer in the archived copy of referecing object to point to
the current archived object.
5. The Klass of the current java object is added to a list of Klasses for
loading and initializing before any object in the archived graph can be
accessed at runtime.
Runtime initialization from archived system module objects
============================================
VM.initializeFromArchive(<class>) is called from ArchivedModuleGraph's static
initializer to initialize from the archived module information. Klasses in the
recorded list are loaded, linked and initialized. The static fields in
ArchivedModuleGraph class instance are initialized using the archived field values.
After initialization, the archived system module objects can be used directly.
If the archived java heap data is not successfully mapped at runtime, or there
is an error during VM.initializeFromArchive(), then all static fields in
ArchivedModuleGraph are not initialized. In that case, system ModuleDescriptor
and ModuleReference objects are created as normal.
In non-CDS mode, VM.initializeFromArchive(<class>) returns immediately with
minimum added overhead for normal execution.
Thanks,
Jiangli
