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

[Volker Simonis]

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 ENABLE_CDS
> >>
> >> 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 '--enable_cds' which
> >> defaults to 'true' on all platforms except AIX.
> >>
> >> The check for '--enable_cds_archive' should use the result of
> >> '--enable_cds' (which is saved in "ENABLE_CDS") 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
>