RFR: 8207263: Store the Configuration for system modules into CDS archive
Claes Redestad
claes.redestad at oracle.com
Thu Aug 9 09:55:05 UTC 2018
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