Review request (s): 7116081: USE_PRECOMPILED_HEADER=0 triggers a single threaded build of the JVM

[Stefan Karlsson]

Looping in build-dev and build-infra-dev.

StefanK

On 11/29/2011 09:47 AM, Stefan Karlsson wrote:
> David,
>
> Thanks for the review.
>
> All,
>
> Updated webrev: http://cr.openjdk.java.net/~stefank/7116081/webrev.2/
>
> The rationale for this change is inlined:
>
> On 11/29/2011 12:48 AM, David Holmes wrote:
>> Hi Stefan,
>>
>> On 29/11/2011 12:34 AM, Stefan Karlsson wrote:
>>> http://cr.openjdk.java.net/~stefank/7116081/webrev/
>>>
>>> Turning off the precompiled headers is somewhat broken. It triggers a
>>> single threaded build even when HOTSPOT_BUILD_JOBS has been set. With
>>> this fix the compile times went from around 14 minutes to 2.5 minutes,
>>> on an 8 core machine.
>>
>> Took me a while to figure out why this was the case :)
>>
>>> This affects both Linux and BSD builds, but has only been tested on
>>> Linux. It would be great if someone with access to a BSD machine could
>>> verify this fix.
>>
>> The problem here was using
>>
>> ifdef USE_PRECOMPILED_HEADER
>>
>> when defining it to 0 is used to turn it off - so it seems to me the 
>> better fix here was to simply change to:
>>
>> ifeq ($(USE_PRECOMPILED_HEADER),1)
>
> I've changed the conditional to check USE_PRECOMPILED_HEADER instead.
>
> While doing that I found another issue. In gcc.make we try to redefine 
> USE_PRECOMPILED_HEADER:
>
>  88 ifneq ($(USE_PRECOMPILED_HEADER),0)
>  89 USE_PRECOMPILED_HEADER=1
>
>
> That doesn't work, so setting USE_PRECOMPILED_HEADER=<arbitrary string 
> (not 0)> on the command line will leave the precompiled headers on, 
> but later logic:
>
>  219 ifneq ($(USE_PRECOMPILED_HEADER),1)
>  220 CFLAGS += -DDONT_USE_PRECOMPILED_HEADER
>
> will empty the precompiled.hpp file. From precompiled.hpp:
>
> // Precompiled headers are turned off for Sun Studion,
> // or if the user passes USE_PRECOMPILED_HEADER=0 to the makefiles.
> #ifndef DONT_USE_PRECOMPILED_HEADER
>
> In the new webrev I've changed all places where we read 
> USE_PRECOMPILED_HEADER to always compare against 0.
>
> thanks,
> StefanK
>
>>
>>
>> The removal of:
>>
>> PrecompiledOption = -DUSE_PRECOMPILED_HEADER
>>
>> seems okay.
>>
>> Cheers,
>> David
>>
>>
>