Hi David, thanks for sponsoring the change!
Best regards, Goetz. > -----Original Message----- > From: David Holmes [mailto:[email protected]] > Sent: Donnerstag, 31. August 2017 23:15 > To: Lindenmaier, Goetz <[email protected]>; 'Magnus Ihse Bursie' > <[email protected]>; hotspot-runtime- > [email protected]; build-dev ([email protected]) <build- > [email protected]> > 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:[email protected]] > >> Sent: Thursday, August 31, 2017 3:35 PM > >> To: David Holmes <[email protected]>; Lindenmaier, Goetz > >> <[email protected]>; [email protected]; > >> build-dev ([email protected]) <[email protected]> > >> 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. > >>>> > >
