RFR(XS): 8211837: Creation of the default CDS Archive should depend on ENABLE_CDS (original) (raw)

Volker Simonis volker.simonis at gmail.com
Tue Oct 9 06:17:53 UTC 2018

Hi Jiangli,

thanks for looking at the change. Unfortunately I've already pushed it in order to fix our nightly builds.

I think the comment should only mention the variants which are explicitly excluded by that function. All other cases are now handled by the "ENABLE_CDS" flag which seems more natural to me. But "ENABLE_CDS" is defined in hotspot.m4 where I put in some comments.

Regards, Volker

On Mon, Oct 8, 2018 at 8:06 PM Jiangli Zhou <jiangli.zhou at oracle.com> wrote:

Hi Volker, Looks good. Thanks for fixing! It would be good to update the following comment in jdk-options.m4 to add AIX, minimal and core. 609 ################################################################################ 610 # 611 # Disable the default CDS archive generation 612 # cross compilation - disabled 613 # zero - off by default (not a tested configuration) 614 #

Thanks! Jiangli On 10/8/18 3:10 AM, Volker Simonis wrote: > Hi Martin, Goetz, > > thanks for reviewing my patch! As Aleksey posted a similar patch for > fixing the Zero build I've extended my patch to handle > zero/minimal/core as well. > > In fact the patch now disables CDS on AIX, minimal, core and zero > unless the user explicitly requests it with the help of > --with-jvm-features=cds. The configuration variant with > --with-jvm-features=-cds should now also be handled correctly (I > hope caught all possible combinations :) > > If CDS is disabled, the creation of the default class list and default > archive will now be disabled as well. > > @Aleksey Shipilev : can you please check if this new version of my > patch also works for you? > > http://cr.openjdk.java.net/~simonis/webrevs/2018/8211837.v1/ > > Thank you and best regards, > Volker > On Mon, Oct 8, 2018 at 11:10 AM Doerr, Martin <martin.doerr at sap.com> wrote: >> Hi Volker, >> >> looks good. Thanks for fixing. >> Of course, it would be great if this could be used to fix minimal/zero build, too. >> >> Best regards, >> Martin >> >> >> -----Original Message----- >> From: hotspot-runtime-dev <hotspot-runtime-dev-bounces at openjdk.java.net> On Behalf Of Volker Simonis >> Sent: Montag, 8. Oktober 2018 10:19 >> To: build-dev <build-dev at openjdk.java.net>; hotspot-runtime-dev at openjdk.java.net runtime <hotspot-runtime-dev at openjdk.java.net> >> Subject: RFR(XS): 8211837: Creation of the default CDS Archive should depend on ENABLECDS >> >> Hi, >> >> can I please have a review for the following trivial build fix: >> >> http://cr.openjdk.java.net/~simonis/webrevs/2018/8211837/ >> https://bugs.openjdk.java.net/browse/JDK-8211837 >> >> It makes no sense to try to create a default CDS archive on VMs which >> don't support CDS at all. We already have '--enablecds' which >> defaults to 'true' on all platforms except AIX. >> >> The check for '--enablecdsarchive' should use the result of >> '--enablecds' (which is saved in "ENABLECDS") and only enable >> default archive creation if CDS is enabled. >> >> Failure to do so currently breaks the AIX build (after JDK-)8202951 was pushed). >> >> Thank you and best regards, >> Volker

More information about the build-dev mailing list