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

[Magnus Ihse Bursie]

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
])


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?

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