RFR: JDK-8062223: Upgrading to ccache 1.3.10 disables the use of ccache

[Erik Joelsson]

On 2015-01-30 11:24, Magnus Ihse Bursie wrote:
> On 2015-01-27 14:23, Erik Joelsson wrote:
>> Hello,
>>
>> Please review this fix cleaning up the ccache setup in configure. 
>> This patch addresses the concerns in the following bugs:
>> https://bugs.openjdk.java.net/browse/JDK-8065791
>> https://bugs.openjdk.java.net/browse/JDK-8014074
>> https://bugs.openjdk.java.net/browse/JDK-8062223
>>
>> Webrev: http://cr.openjdk.java.net/~erikj/8062223/webrev.root.01/
>
> I would prefer if you rewrote the code in build-performance.m4 slightly.
>
> We have an anti-pattern that's unfortunately too common in our 
> autoconf scripts, but I'm trying to eradicate it. :-)
>
> The thing to note is that AC_ARG_ENABLE() is not affected by "if". So 
> even when you write:
>  162   if test "x$TOOLCHAIN_TYPE" = "xgcc" -o "x$TOOLCHAIN_TYPE" = 
> "xclang"; then
>  163     AC_ARG_ENABLE([ccache],
>  164         [AS_HELP_STRING([--enable-ccache],
>  165         [enable using ccache to speed up recompilations 
> @<:@disabled@:>@])])
>
> the argument --enable-ccache is available for all toolchains. This is 
> a good thing, since we don't want to make options becoming unknown 
> (and thus fail) depending on circumstances. However, this behavior 
> cannot be deduced from the code, where it looks like it is conditional.
>
> A better pattern is something like this:
> AC_DEFUN([HANDLE_MY_OPT]), ([
>   # Always start by declaring the option
>   AC_ARG_ENABLE([myopt], ...)
>
>   if test SUPPORTED_PLATFORM; then
>    # .. handle option
>   else
>     AC_MSG_WARN([--myopt is ignored on this platform])
>    # or ERROR if more appropriate
>   fi
> ])
>
Right, that should be fixed.
>
> Also, you now sets
> CCACHE_SLOPPINESS=pch_defines,time_macros
> only if -fpch-preprocess works with the compiler. But if it don't, you 
> should probably set
> CCACHE_SLOPPINESS=time_macros
> to keep the old behavior. Unless time_macros makes no sense if not 
> -fpch-preprocess is enabled?
>
time_macros was set only for precompiled headers. With newer versions of 
ccache you also need to set pch_defines. There is no need for either if 
not using precompiled headers.

/Erik
> Otherwise it looks good!
>
> /Magnus
>
>>
>> I changed the test for ccache version to explicitly check for known 
>> older versions that we don't want to use with precompiled headers. 
>> The test is only done if precompiled headers are enabled, since 
>> without precompiled headers, we don't know of any issues with ccache.
>>
>> I fixed the ccache options needed to fully function with precompiled 
>> headers in hotspot. These options will now be used if precompiled 
>> headers are enabled. They currently aren't fully used.
>>
>> I updated the documentation to reflect our current stance on ccache.
>>
>> As noted in JDK-8014074, ccache works better without precompiled 
>> headers, so I recommend turning it off when using ccache. (I actually 
>> recommend turning it off on linux regardless.)
>>
>> Some numbers building just hotspot ("make hotspot") from my machine 
>> (E5-2665 @ 2.4GHz, 16 cores + HT) with JOBS=15 which is default:
>>
>> Product/release with precompiled headers
>> no ccache: 03:13
>> empty cache: 03:37
>> perfect cache: 01:36
>>
>> Product/release without precompiled headers
>> no ccache: 02:35
>> empty cache: 02:43
>> perfect cache: 01:04
>>
>> /Erik
>