Define CCACHE_CPP2 when using ccache and Clang

RESOLVED FIXED in mozilla17

Status

()

Core
Build Config
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: gps, Assigned: espindola)

Tracking

Trunk
mozilla17
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
We should probably define CCACHE_CPP2 when using ccache and Clang. See bug 754988.
(Reporter)

Comment 1

5 years ago
As part of this, it would also be nice to tackle the general problem of not being able to pass the various CCACHE_* environment variables to ccache. Currently, if you add an export to your .mozconfig (e.g. "export CCACHE_BASEDIR=/Users/gps"), this has no effect (at least if you execute through client.mk - maybe if you launch the Makefile in the objdir directly it works). Furthermore, you can't ac_add_options --with-ccache="CCACHE_BASEDIR=/Users/gps /usr/local/bin/ccache" because configure is testing -e and -z on the string value.

Perhaps we could solve this by adding CC_PRE and CXX_PRE or something similar? That would also give us the backdoor to easily proxy all compile commands through another script (e.g. something that intercepted compiler warning output and aggregated information or Clang's scan-build tools). In the current world, getting complex control requires disabling ccache. Just a thought.
No longer blocks: 574346
(Reporter)

Comment 2

5 years ago
I don't have any specifically quantitative evidence to back this up, but it seems that setting CCACHE_CPP2 results in my ccache filling up quicker. We should probably measure this and run it past RelEng before turning this on by default.
(Reporter)

Comment 3

5 years ago
(In reply to Gregory Szorc [:gps] from comment #2)
> I don't have any specifically quantitative evidence to back this up, but it
> seems that setting CCACHE_CPP2 results in my ccache filling up quicker. We
> should probably measure this and run it past RelEng before turning this on
> by default.

Upon further analysis, the results appear to be similar to non-CPP2 builds. Maybe it was CCACHE_BASEDIR (which I set at the same time) affecting things.
Here's a trick I chanced upon:
export CC="env ENV_VAR=foo clang"
Created attachment 643582 [details] [diff] [review]
Define CCACHE_CPP2 when using ccache and Clang
Assignee: nobody → respindola
Status: NEW → ASSIGNED
Attachment #643582 - Flags: review?(khuey)
Attachment #643582 - Flags: feedback?(gps)
(Reporter)

Comment 6

5 years ago
Comment on attachment 643582 [details] [diff] [review]
Define CCACHE_CPP2 when using ccache and Clang

Review of attachment 643582 [details] [diff] [review]:
-----------------------------------------------------------------

This is beyond my comfort to give feedback on.

Mike Hommey has been doing a lot of work on autoconf foo lately and might have some things to say though.
Attachment #643582 - Flags: feedback?(gps)
Blocks: 775305
Attachment #643582 - Flags: feedback?(mh+mozilla)
I did some simple benchmarking of my laptop:

ccache build 1
real	36m40.916s
user	99m18.844s
sys	12m22.869s

ccache build 2
real	11m53.582s
user	9m9.587s
sys	3m22.222s

no ccache
real	32m11.518s
user	90m58.849s
sys	8m43.691s

Which suggests it is worth enabling, but really depends on the cache hit rate we have on try. Do we have any numbers on that?
p.s.: I used a 10GB cache on that benchmark.
Comment on attachment 643582 [details] [diff] [review]
Define CCACHE_CPP2 when using ccache and Clang

Review of attachment 643582 [details] [diff] [review]:
-----------------------------------------------------------------

::: build/autoconf/compiler-opts.m4
@@ +206,5 @@
>          _WARNINGS_CXXFLAGS="${_WARNINGS_CXXFLAGS} $1$2"
>      fi
>  ])
> +
> +AC_DEFUN([MOZ_TOOL_VARIABLES],

I'd move that in a separate toolchain.m4 file.
Attachment #643582 - Flags: review?(khuey)
Attachment #643582 - Flags: review+
Attachment #643582 - Flags: feedback?(mh+mozilla)
https://hg.mozilla.org/mozilla-central/rev/3e4e9e518bef
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
You need to log in before you can comment on or make changes to this bug.