RFR: 8207263: Store the Configuration for system modules into CDS archive
Jiangli Zhou
jiangli.zhou at oracle.com
Thu Aug 9 18:42:06 UTC 2018
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
>>>>>>>
>>>>>
>>>>
>>>
>>
>
More information about the jigsaw-dev
mailing list