RFR: 8207263: Store the Configuration for system modules into CDS archive

[Jiangli Zhou]

Thanks, Ioi!

Jiangli


On 8/9/18 4:06 PM, Ioi Lam wrote:
> Looks good to me, too. Thanks!
>
> - Ioi
>
>
> On 8/9/18 12:13 PM, Claes Redestad wrote:
>> Updates looks good, thanks!
>>
>> /Claes
>>
>> On 2018-08-09 20:42, Jiangli Zhou wrote:
>>> Here is the updated webrev with comment changes suggested by Claes:
>>>
>>> http://cr.openjdk.java.net/~jiangli/8207263/webrev.02/
>>>
>>> Updated files:
>>> http://cr.openjdk.java.net/~jiangli/8207263/webrev.02/src/java.base/share/classes/java/util/ImmutableCollections.java.frames.html 
>>>
>>> http://cr.openjdk.java.net/~jiangli/8207263/webrev.02/src/java.base/share/classes/jdk/internal/module/ArchivedModuleGraph.java.frames.html 
>>>
>>>
>>> Thanks,
>>> Jiangli
>>>
>>> On 8/9/18 8:50 AM, Jiangli Zhou wrote:
>>>> Hi Claes,
>>>>
>>>> Thanks for reviewing the library and test changes! I'll remove the 
>>>> comments that you pointed below from ImmutableCollections and 
>>>> update the CheckArchivedModule test.
>>>>
>>>> Thanks!
>>>>
>>>> Jiangli
>>>>
>>>>
>>>> On 8/9/18 2:55 AM, Claes Redestad wrote:
>>>>> Library and test changes look good to me. Nits:
>>>>>
>>>>> ImmutableCollections:
>>>>>
>>>>> - // Initialize EMPTY_FOO from the archive comments feel redundant
>>>>>
>>>>> CheckArchivedModule:
>>>>>
>>>>> - perhaps call out that checking identity is necessary and 
>>>>> intentional
>>>>>
>>>>> /Claes
>>>>>
>>>>> On 2018-08-09 01:43, Jiangli Zhou wrote:
>>>>>> Thanks Claes for resolving JDK-8209003 quickly. Here is my 
>>>>>> updated weberv that includes following changes:
>>>>>>
>>>>>> - Added archiving for singletons in ImmutableCollections 
>>>>>> (ListN.EMPTY_LIST, SetN.EMPTY_SET and MapN.EMPTY_MAP). Please see 
>>>>>> changes in ImmutableCollections.java.
>>>>>> - Incorporated Calvin and Ioi feedbacks on vmSymbols.hpp.
>>>>>> - Added a new test case to check the parents and modules of 
>>>>>> archived EMPTY_CONFIGURATION.
>>>>>> - Added argument length check in CheckArchivedModuleApp.java test 
>>>>>> as Calvin suggested.
>>>>>>
>>>>>> http://cr.openjdk.java.net/~jiangli/8207263/webrev.01/
>>>>>>
>>>>>> Thanks,
>>>>>> Jiangli
>>>>>>
>>>>>> On 8/6/18 9:48 AM, Jiangli Zhou wrote:
>>>>>>> Hi Calvin and Ioi,
>>>>>>>
>>>>>>> Thanks for reviewing the change! I'll incorporate your suggestions.
>>>>>>>
>>>>>>> As Claes pointed out in his email, there was a subtle issue with 
>>>>>>> the empty configuration, which was undetected by the testing for 
>>>>>>> archiving changes but could introduce a bug in certain cases. 
>>>>>>> Claes has already filed JDK-8209003 for consolidating empty 
>>>>>>> collections usage in module code. I'll look into archiving the 
>>>>>>> immutable singletons in java.util.ImmutableCollections.
>>>>>>>
>>>>>>> Thanks!
>>>>>>>
>>>>>>> Jiangli
>>>>>>>
>>>>>>>
>>>>>>> On 8/3/18 2:58 PM, Ioi Lam wrote:
>>>>>>>> On 8/3/18 11:37 AM, Calvin Cheung wrote:
>>>>>>>>> Hi Jiangli,
>>>>>>>>>
>>>>>>>>> The changes look good to me.
>>>>>>>>>
>>>>>>>>> I have couple of minor comments:
>>>>>>>>>
>>>>>>>>> 1) vmSymbols.hpp
>>>>>>>>>
>>>>>>>>>  653   template(url_void_signature, "(Ljava/net/URL;)V") \
>>>>>>>>>  654   template(toFileURL_name, "toFileURL") \
>>>>>>>>>  655   template(toFileURL_signature, 
>>>>>>>>> "(Ljava/lang/String;)Ljava/net/URL;")
>>>>>>>>>
>>>>>>>>> Since you've moved the above lines to after 
>>>>>>>>> “template(systemModules_signature, …”, I’d suggest rearrange 
>>>>>>>>> the entire block (lines 652 - 659) in alphabetical order.
>>>>>>>>>
>>>>>>>> Hi Jiangli,
>>>>>>>>
>>>>>>>> I've reviewed the code. It looks like a clean change and it's 
>>>>>>>> great to make further progress in start-up improvement!
>>>>>>>>
>>>>>>>> Just a small note on vmSymbols.hpp:  this line can be deleted 
>>>>>>>> because the symbol is no longer used.
>>>>>>>>
>>>>>>>>   template(jdk_vm_cds_SharedClassInfo, 
>>>>>>>> "jdk/vm/cds/SharedClassInfo")
>>>>>>>>
>>>>>>>> Thanks
>>>>>>>> - Ioi
>>>>>>>>
>>>>>>>>> 2) CheckArchivedModuleApp.java
>>>>>>>>>
>>>>>>>>> Since it now expects two input args, I’d suggest checking the 
>>>>>>>>> number of input args and throw an exception if it is not equal 
>>>>>>>>> to two.
>>>>>>>>>
>>>>>>>>> thanks,
>>>>>>>>> Calvin
>>>>>>>>>
>>>>>>>>> On 7/20/18, 11:31 AM, Jiangli Zhou wrote:
>>>>>>>>>> Please review the following webrev that archives the system 
>>>>>>>>>> module boot layer Configuration (including all java objects 
>>>>>>>>>> reachable from the Configuration) in CDS archive. This is 
>>>>>>>>>> built on top of the earlier change for JDK-8202035 
>>>>>>>>>> (https://bugs.openjdk.java.net/browse/JDK-8202035), which 
>>>>>>>>>> provides a framework for object sub-graph archiving.
>>>>>>>>>>
>>>>>>>>>> The boot layer Configuration is created in 
>>>>>>>>>> ModuleBootstrap.boot() (similar to the archived system 
>>>>>>>>>> ModuleDescriptor objects, etc) and is unchanged after 
>>>>>>>>>> construction. With archived boot layer Configuration, it 
>>>>>>>>>> allows runtime to bypass the work for creating the 
>>>>>>>>>> configuration. Currently, this is only supported when the 
>>>>>>>>>> initial module is unnamed module. Measurements indicate 
>>>>>>>>>> archiving the boot layer Configuration improves the startup 
>>>>>>>>>> time by 1% ~ 1.5% (on linux-x64) when running HelloWorld from 
>>>>>>>>>> -cp at runtime.
>>>>>>>>>>
>>>>>>>>>> Many thanks to Alan and Claes for discussions and 
>>>>>>>>>> contributions to this change!
>>>>>>>>>>
>>>>>>>>>> Webrev: http://cr.openjdk.java.net/~jiangli/8207263/webrev.00/
>>>>>>>>>>
>>>>>>>>>> RFE: https://bugs.openjdk.java.net/browse/JDK-8207263
>>>>>>>>>>
>>>>>>>>>> Tested with tier1 - tier5 tests via mach5.
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>>
>>>>>>>>>> Jiangli
>>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>