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

Magnus Ihse Bursie magnus.ihse.bursie at oracle.com
Thu Aug 31 13:35:26 UTC 2017



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.
>>




More information about the build-dev mailing list