RFR(M): 8186978: Introduce configure argument enable-cds

[Lindenmaier, Goetz]

Hi David, 

thanks for sponsoring the change!

Best regards,
  Goetz.

> -----Original Message-----
> From: David Holmes [mailto:david.holmes at oracle.com]
> Sent: Donnerstag, 31. August 2017 23:15
> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; 'Magnus Ihse Bursie'
> <magnus.ihse.bursie at oracle.com>; hotspot-runtime-
> dev at openjdk.java.net; build-dev (build-dev at openjdk.java.net) <build-
> dev at openjdk.java.net>
> Subject: Re: RFR(M): 8186978: Introduce configure argument enable-cds
> 
> Hi Goetz,
> 
> I will sponsor this.
> 
> Thanks,
> David
> 
> On 1/09/2017 12:49 AM, Lindenmaier, Goetz wrote:
> > Hi,
> >
> > thanks for reviewing everybody!
> > Yes, works fine without that assignment. New webrev:
> > http://cr.openjdk.java.net/~goetz/wr17/8186978-disableCDS/webrev.02/
> >
> > Could someone please sponsor? I think autogen.sh needs to be run
> > before submitting.
> >
> > Best regards,
> >    Goetz.
> >
> >> -----Original Message-----
> >> From: Magnus Ihse Bursie [mailto:magnus.ihse.bursie at oracle.com]
> >> Sent: Thursday, August 31, 2017 3:35 PM
> >> To: David Holmes <david.holmes at oracle.com>; Lindenmaier, Goetz
> >> <goetz.lindenmaier at sap.com>; hotspot-runtime-dev at openjdk.java.net;
> >> build-dev (build-dev at openjdk.java.net) <build-dev at openjdk.java.net>
> >> Subject: Re: RFR(M): 8186978: Introduce configure argument enable-cds
> >>
> >>
> >>
> >> On 2017-08-31 14:47, David Holmes wrote:
> >>> Hi Goetz,
> >>>
> >>> On 31/08/2017 10:29 PM, Lindenmaier, Goetz wrote:
> >>>> Hi,
> >>>>
> >>>> Tests for class data sharing (cds) are enabled if @requires vm.cds is
> >>>> true.
> >>>> The property vm.cds depends on the preprocessor macro
> ENABLE_CDS.
> >> ... but you mean INCLUDE_CDS. :-)
> >>
> >>>> This can not yet be switched by configure. It's only disabled
> >>>> automatically
> >>>> for the minimal build.
> >>>>
> >>>> This change introduces enable-cds with default true, which only takes
> >>>> effect
> >>>> in the non-minimal build. If disabled, generate-classlist is
> >>>> disabled, too.
> >>>>
> >>>> Please review this change. I please need a sponsor.
> >>>> http://cr.openjdk.java.net/~goetz/wr17/8186978-
> >> disableCDS/webrev.01/index.html
> >>>>
> >>>
> >>> I'll let the build guys comment in detail, but the structure for this
> >>> doesn't quite look right to me. I don't understand why you have in
> >>> spec.gmk.in:
> >>>
> >>> + ENABLE_CDS:=@ENABLE_CDS@
> >>>
> >>> when in the hotspot build CDS is controlled via the feature setting:
> >>>
> >>> ifneq ($(call check-jvm-feature, cds), true)
> >>>
> >>> which you are already handling. ??
> >>
> >> Agree, the ENABLE_CDS variable is only used internally in the configure
> >> script and need not/should not be exported in spec.gmk.in. As David
> >> says, the test ($(call check-jvm-feature, cds), true) is enough to
> >> determine if to send the -DINCLUDE_CDS to the compiler.
> >>
> >> Just remove the changes to spec.gmk.in, and I'm ok with the patch.
> >>
> >> /Magnus
> >>
> >>
> >>>
> >>> Thanks,
> >>> David
> >>>
> >>>
> >>>> Best regards,
> >>>>     Goetz.
> >>>>
> >