RFR(L): 8202035: Archive the set of ModuleDescriptor and ModuleReference objects for system modules
Ioi Lam
ioi.lam at oracle.com
Fri Jul 6 20:36:05 UTC 2018
Hi Jiangli,
The VM changes look good to me.
For the tests: I think we need a comment here saying that "mods" is
intentionally empty, and also an explanation why it's not necessary to
actually fill with actual modules?
Thanks
- Ioi
>> 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.
>
On 7/6/18 12:34 PM, Jiangli Zhou wrote:
> 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.cheung at 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
>>>
>>>
More information about the core-libs-dev
mailing list