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

[Jiangli Zhou]

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
>>>>>>
>>>>
>>>
>>
>