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

Fri Oct 5 16:57:05 UTC 2018

Dear all,

JEP 341, Default CDS Archive has been targeted to JDK 12 (please see 
Mark's email from yesterday, Re: JEP proposed to target JDK 12: 341: 
Default CDS Archives). Code review, testing and performance runs are 
completed for the default CDS archive changes. I've started the final 
pre-integration test yesterday with all tier1 - tier8. So far most of 
the tiers have been completed. Just a heads up, I'm planning to 
integrate the change in the next few hour.

Thanks everyone for contributing to this effort!!!

Jiangli

On 9/6/18 12:45 PM, Jiangli Zhou wrote:
> Hi,
>
> The JEP (http://openjdk.java.net/jeps/341) for the default CDS 
> archives is now a candidate. Please see Mark's email, 'New candidate 
> JEP: 341: Default CDS Archives'. Please help test Erik's makefile 
> patch for your platform if it is not built/tested in openjdk mainline 
> to prevent any possible breakage on your side. Any comments/feedbacks 
> on the default CDS archive are highly appreciated!
>
>   http://cr.openjdk.java.net/~jiangli/8202951/webrev.02/
>
> The above webrev is sync'ed with the lasted jdk/jdk repository today.
>
> Thanks!
>
> Jiangli
>
> On 8/30/18 11:26 AM, Jiangli Zhou wrote:
>> Hi Magnus,
>>
>> Sounds good. Will update the message.
>>
>> Thanks!
>>
>> Jiangli
>>
>>
>> On 8/30/18 3:56 AM, Magnus Ihse Bursie wrote:
>>> On 2018-08-29 17:45, 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 see. I still think it would have made more sense, in that case, to 
>>> set the default to false if using zero, but allow override. But I'm 
>>> not going to insist on that.
>>>
>>> However, if the problem with zero is not that it's technically 
>>> impossible, just that it's not tested, I think the message should be 
>>> changed from "[no, not possible with JVM variant zero]" to just 
>>> "[no]" with a comment in the configure script that it's off by 
>>> default since it's not tested.
>>>
>>> Apart from that, Jiangli's latest webrev looks good.
>>>
>>> Jiangli: If you fix it like I suggested, you do not need to respin 
>>> the webrev.
>>>
>>> /Magnus
>>>
>>>>
>>>> /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
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>