[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-hotspot-runtime-dev
Subject:    Re: RFR(M): 8186978: Introduce configure argument enable-cds
From:       David Holmes <david.holmes () oracle ! com>
Date:       2017-08-31 21:15:04
Message-ID: bd33f88b-df1d-3feb-687a-c7169eaf2aa1 () oracle ! com
[Download RAW message or body]

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@oracle.com]
>> Sent: Thursday, August 31, 2017 3:35 PM
>> To: David Holmes <david.holmes@oracle.com>; Lindenmaier, Goetz
>> <goetz.lindenmaier@sap.com>; hotspot-runtime-dev@openjdk.java.net;
>> build-dev (build-dev@openjdk.java.net) <build-dev@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.
>>>>
> 
[prev in list] [next in list] [prev in thread] [next in thread]