RFR: JDK-8202951: Implementation of JEPJDK-8204247: Include default CDS (Class Data Sharing) archive in JDK binary

Jiangli Zhou jiangli.zhou at oracle.com
Wed Aug 29 21:45:14 UTC 2018


Hi Erik and Magnus,

I updated Images.gmk accordingly. Magnus, thanks for reviewing!

http://cr.openjdk.java.net/~jiangli/8202951/webrev.02/make/Images.gmk.frames.html

Complete webrev: http://cr.openjdk.java.net/~jiangli/8202951/webrev.02/

Please see comments below.

On 8/29/18 8:45 AM, Erik Joelsson wrote:
>
> Hello Magnus,
>
> As the author of the build changes I will answer this.
>
> On 2018-08-29 04:53, Magnus Ihse Bursie wrote:
>> On 2018-08-28 18:25, Erik Joelsson wrote:
>>> Build changes look good to me (but should probably get review from 
>>> someone else).
>>
>> I'm (as usually) not as happy as Erik. ;-)
>>
>> In Images.gmk, you have added this rule:
>>           $@ -Xshare:dump -Xmx128M -Xms128M $(LOG_INFO)
>>
>> It took me a while to grasp. You are relying on $(JIMAGE_TARGET_FILE) 
>> to evaluate to bin/java. But that is only incidentally so. This file 
>> is just picked arbitrarily to represent when the entire image is 
>> done, for make. Please use $(JRE_IMAGE_DIR)/bin/java instead.
>>
> I can agree with this part. That was a bit of a hack done in a hurry.
>> The logic in jdk-options.m4 is broken. If indeed it is not possible 
>> to use cds archive with zero, then things will break since it will 
>> still be built if using --enable-cds-archive!
>>
>> What you should do is to set default to true unless using cross 
>> compilation or zero. If the user explicitly sets 
>> --enable-cds-archive, and it's not possible, a fatal error should be 
>> generated.
>>
> Here I'm not as sure. I deliberately let it be possible to override 
> the default behavior for zero as someone might want to fix that at 
> some point, you never know. For cross compilation it's just not 
> possible ever so that's different. Maybe my reasoning is invalid.
I think that's reasonable.

Thanks!
Jiangli
>
> /Erik
>> Apart from this, I'm more on Erik's line. :-)
>>
>> /Magnus
>>
>>
>>
>>>
>>> /Erik
>>>
>>>
>>> On 2018-08-27 13:33, Jiangli Zhou wrote:
>>>> Please review the implementation for JEP JDK-8204247 
>>>> (https://bugs.openjdk.java.net/browse/JDK-8204247). The goal of the 
>>>> JEP is to include a default CDS archive in JDK 12 binary 
>>>> distribution (downloadable from http://jdk.java.net/12/). The 
>>>> default CDS archive is generated using the default classlist 
>>>> (resides in the lib/ directory) at JDK build time. Any 
>>>> comments/suggestions are highly appreciated.
>>>>
>>>> All makefile changes in the webrev are contributed by Erik Joelsson 
>>>> (many thanks!!).
>>>>
>>>> This is a combination of efforts from different teams and 
>>>> individuals. Thanks to everyone who has been involved in the JEP & 
>>>> implementation discussions, testing and bug fixing!
>>>>
>>>>   JEP: https://bugs.openjdk.java.net/browse/JDK-8204247
>>>>   RFE: https://bugs.openjdk.java.net/browse/JDK-8202951
>>>>   webrev: http://cr.openjdk.java.net/~jiangli/8202951/webrev.00/
>>>>
>>>> Two sanity test cases for the default CDS archive are included 
>>>> test/hotspot/jtreg/runtime/SharedArchiveFile. They are not intended 
>>>> for in-depth CDS functional testing, which is already covered by 
>>>> the existing cds/appcds tests and all tiered tests executing with 
>>>> the default CDS archive enabled.
>>>>
>>>> As part of the webrev, 
>>>> test/jdk/javax/imageio/plugins/png/ItxtUtf8Test.java is also fixed 
>>>> to use larger java heap (JDK-8209739
>>>> , https://bugs.openjdk.java.net/browse/JDK-8209739).
>>>>
>>>> Tests executed:
>>>>  - several rounds of tier1 - tier8 via mach5
>>>>  - JCK lang, api and vm tests via mach5
>>>>
>>>>
>>>> Thanks!
>>>> Calvin, Ioi, Jiangli
>>>>
>>>>
>>>
>>
>



More information about the jdk-dev mailing list