Re: RFR: 8193213 & 8182731: Make the UseAppCDS option obsolete

2018-04-27 Thread Jiangli Zhou
> On Apr 27, 2018, at 1:43 AM, David Holmes wrote: > > On 27/04/2018 4:20 AM, Jiangli Zhou wrote: >>> On Apr 25, 2018, at 10:09 PM, David Holmes wrote: >>> >>> On 26/04/2018 2:34 PM, Jiangli Zhou wrote: Here is the incremental webrev with updates that incorporate all feedbacks: http:

Re: RFR: 8193213 & 8182731: Make the UseAppCDS option obsolete

2018-04-27 Thread David Holmes
On 27/04/2018 4:20 AM, Jiangli Zhou wrote: On Apr 25, 2018, at 10:09 PM, David Holmes wrote: On 26/04/2018 2:34 PM, Jiangli Zhou wrote: Here is the incremental webrev with updates that incorporate all feedbacks: http://cr.openjdk.java.net/~jiangli/8193213_8182731/webrev_inc.02/ Looks good.

Re: RFR: 8193213 & 8182731: Make the UseAppCDS option obsolete

2018-04-26 Thread Jiangli Zhou
> On Apr 26, 2018, at 12:21 PM, Calvin Cheung wrote: > > > > On 4/26/18, 12:00 PM, Jiangli Zhou wrote: >> Hi Calvin, >> >>> On Apr 26, 2018, at 10:10 AM, Calvin Cheung >>> wrote: >>> >>> >>> >>> On 4/25/18, 9:34 PM, Jiangli Zhou wrote: Here is the incremental webrev with updates tha

Re: RFR: 8193213 & 8182731: Make the UseAppCDS option obsolete

2018-04-26 Thread Calvin Cheung
On 4/26/18, 12:00 PM, Jiangli Zhou wrote: Hi Calvin, On Apr 26, 2018, at 10:10 AM, Calvin Cheung wrote: On 4/25/18, 9:34 PM, Jiangli Zhou wrote: Here is the incremental webrev with updates that incorporate all feedbacks: http://cr.openjdk.java.net/~jiangli/8193213_8182731/webrev_inc.02/

Re: RFR: 8193213 & 8182731: Make the UseAppCDS option obsolete

2018-04-26 Thread Jiangli Zhou
Hi Calvin, > On Apr 26, 2018, at 10:10 AM, Calvin Cheung wrote: > > > > On 4/25/18, 9:34 PM, Jiangli Zhou wrote: >> Here is the incremental webrev with updates that incorporate all feedbacks: >> http://cr.openjdk.java.net/~jiangli/8193213_8182731/webrev_inc.02/ > Looks good. Thanks! >> >> -

Re: RFR: 8193213 & 8182731: Make the UseAppCDS option obsolete

2018-04-26 Thread Jiangli Zhou
Thanks, Magnus and Erik! Jiangli > On Apr 26, 2018, at 8:55 AM, Erik Joelsson wrote: > > Build looks good, thanks. > > /Erik > > > On 2018-04-25 21:34, Jiangli Zhou wrote: >> Here is the incremental webrev with updates that incorporate all feedbacks: >> http://cr.openjdk.java.net/~jiangli/81

Re: RFR: 8193213 & 8182731: Make the UseAppCDS option obsolete

2018-04-26 Thread Jiangli Zhou
> On Apr 25, 2018, at 10:09 PM, David Holmes wrote: > > On 26/04/2018 2:34 PM, Jiangli Zhou wrote: >> Here is the incremental webrev with updates that incorporate all feedbacks: >> http://cr.openjdk.java.net/~jiangli/8193213_8182731/webrev_inc.02/ > > Looks good. Thanks! > > One additional c

Re: RFR: 8193213 & 8182731: Make the UseAppCDS option obsolete

2018-04-26 Thread Calvin Cheung
On 4/25/18, 9:34 PM, Jiangli Zhou wrote: Here is the incremental webrev with updates that incorporate all feedbacks: http://cr.openjdk.java.net/~jiangli/8193213_8182731/webrev_inc.02/ Looks good. - Filed JDK-8202282 (https://bugs.openjdk.java.net/browse/JDK-8202282) for TestCommon.makeComma

Re: RFR: 8193213 & 8182731: Make the UseAppCDS option obsolete

2018-04-26 Thread Erik Joelsson
Build looks good, thanks. /Erik On 2018-04-25 21:34, Jiangli Zhou wrote: Here is the incremental webrev with updates that incorporate all feedbacks: http://cr.openjdk.java.net/~jiangli/8193213_8182731/webrev_inc.02/ - Filed JDK-8202282 (https://bugs.openjdk.java.net/browse/JDK-8202282) for T

Re: RFR: 8193213 & 8182731: Make the UseAppCDS option obsolete

2018-04-26 Thread Magnus Ihse Bursie
On 2018-04-26 06:34, Jiangli Zhou wrote: Here is the incremental webrev with updates that incorporate all feedbacks: http://cr.openjdk.java.net/~jiangli/8193213_8182731/webrev_inc.02/ Looks even better. /Magnus - Filed JDK-8202282 (https://bugs.openjdk.java.net/browse/JDK-8202282) for TestCo

Re: RFR: 8193213 & 8182731: Make the UseAppCDS option obsolete

2018-04-25 Thread David Holmes
On 26/04/2018 2:34 PM, Jiangli Zhou wrote: Here is the incremental webrev with updates that incorporate all feedbacks: http://cr.openjdk.java.net/~jiangli/8193213_8182731/webrev_inc.02/ Looks good. One additional comment on test/hotspot/jtreg/runtime/appcds/SharedArchiveFile.java - it seems

Re: RFR: 8193213 & 8182731: Make the UseAppCDS option obsolete

2018-04-25 Thread Jiangli Zhou
Here is the incremental webrev with updates that incorporate all feedbacks: http://cr.openjdk.java.net/~jiangli/8193213_8182731/webrev_inc.02/ - Filed JDK-8202282 (https://bugs.openjdk.java.net/browse/JDK-8202282) for TestCommon.makeCommandLineForAppCDS() cleanup. - Removed case 2, 3 and 4 in Sha

Re: RFR: 8193213 & 8182731: Make the UseAppCDS option obsolete

2018-04-25 Thread Jiangli Zhou
Hi David, > On Apr 25, 2018, at 3:12 PM, David Holmes wrote: > > Hi Jiangli, > > On 26/04/2018 3:39 AM, Jiangli Zhou wrote: >> Hi David, >> Thanks a lot for the review! >>> On Apr 24, 2018, at 11:17 PM, David Holmes wrote: >>> >>> cc'ing build-dev for the makefile change >>> >>> Hi Jiangli,

Re: RFR: 8193213 & 8182731: Make the UseAppCDS option obsolete

2018-04-25 Thread David Holmes
Hi Jiangli, On 26/04/2018 3:39 AM, Jiangli Zhou wrote: Hi David, Thanks a lot for the review! On Apr 24, 2018, at 11:17 PM, David Holmes wrote: cc'ing build-dev for the makefile change Hi Jiangli, On 25/04/2018 10:53 AM, Jiangli Zhou wrote: Please review the following changes that stream

Re: RFR: 8193213 & 8182731: Make the UseAppCDS option obsolete

2018-04-25 Thread Jiangli Zhou
Hi Erik, > On Apr 25, 2018, at 8:30 AM, Erik Joelsson wrote: > > Hello, > > I would prefer if the .tmp file was not deleted. That will make it easier to > debug problems in the future. We have recently had reason to look at the > contents of the files here to figure out if the generator ran p

Re: RFR: 8193213 & 8182731: Make the UseAppCDS option obsolete

2018-04-25 Thread Jiangli Zhou
Hi Magnus, Thanks a lot for the review! Jiangli > On Apr 25, 2018, at 2:17 AM, Magnus Ihse Bursie > wrote: > > > > On 2018-04-25 08:17, David Holmes wrote: >> cc'ing build-dev for the makefile change >> >> Hi Jiangli, >> >> On 25/04/2018 10:53 AM, Jiangli Zhou wrote: >>> Please review the

Re: RFR: 8193213 & 8182731: Make the UseAppCDS option obsolete

2018-04-25 Thread Jiangli Zhou
Hi David, Thanks a lot for the review! > On Apr 24, 2018, at 11:17 PM, David Holmes wrote: > > cc'ing build-dev for the makefile change > > Hi Jiangli, > > On 25/04/2018 10:53 AM, Jiangli Zhou wrote: >> Please review the following changes that streamline the support for >> application class

Re: RFR: 8193213 & 8182731: Make the UseAppCDS option obsolete

2018-04-25 Thread Ioi Lam
Hi Jiangli, The code changes look good to me. I agree with David's comments about the test cases. Thanks - Ioi On 4/24/18 11:17 PM, David Holmes wrote: cc'ing build-dev for the makefile change Hi Jiangli, On 25/04/2018 10:53 AM, Jiangli Zhou wrote: Please review the following changes tha

Re: RFR: 8193213 & 8182731: Make the UseAppCDS option obsolete

2018-04-25 Thread Erik Joelsson
Hello, I would prefer if the .tmp file was not deleted. That will make it easier to debug problems in the future. We have recently had reason to look at the contents of the files here to figure out if the generator ran properly. If leaving it around, then perhaps call it .raw or something les

Re: RFR: 8193213 & 8182731: Make the UseAppCDS option obsolete

2018-04-25 Thread Magnus Ihse Bursie
On 2018-04-25 08:17, David Holmes wrote: cc'ing build-dev for the makefile change Hi Jiangli, On 25/04/2018 10:53 AM, Jiangli Zhou wrote: Please review the following changes that streamline the support for application class data sharing by eliminating the requirement of using -XX:+UseAppCDS

Re: RFR: 8193213 & 8182731: Make the UseAppCDS option obsolete

2018-04-24 Thread David Holmes
cc'ing build-dev for the makefile change Hi Jiangli, On 25/04/2018 10:53 AM, Jiangli Zhou wrote: Please review the following changes that streamline the support for application class data sharing by eliminating the requirement of using -XX:+UseAppCDS to enable the feature. The support for app