build: Enhance Ccache performance across worktrees and build trees

hebasto commented at 11:56 am on September 10, 2024: member

To maximize performance, it is essential to follow the recommendations in the Productivity Notes: set the base_dir Ccache configuration option or use the CCACHE_BASEDIR environment variable:

0$ export CCACHE_BASEDIR=$HOME

Here are examples of usage on Ubuntu 24.04:

Across different worktrees:

 0$ export CCACHE_DIR=$(mktemp -d)
 1$ git worktree add ../wt_1 4d3f360e2af94f4ed46dc0943196d548da82003e
 2$ pushd ../wt_1
 3$ cmake -B build -DWITH_CCACHE=ON
 4$ cmake --build build -t bitcoind -j $(nproc)
 5$ popd
 6$ git worktree add ../wt_2 4d3f360e2af94f4ed46dc0943196d548da82003e
 7$ pushd ../wt_2
 8$ cmake -B build -DWITH_CCACHE=ON
 9$ ccache --zero-stats
10$ cmake --build build -t bitcoind -j $(nproc)
11$ popd
12$ ccache --show-stats
13Cacheable calls:    302 / 302 (100.0%)
14  Hits:             302 / 302 (100.0%)
15    Direct:         302 / 302 (100.0%)
16    Preprocessed:     0 / 302 ( 0.00%)
17  Misses:             0 / 302 ( 0.00%)
18Local storage:
19  Cache size (GiB): 0.2 / 5.0 ( 3.31%)
20  Hits:             302 / 302 (100.0%)
21  Misses:             0 / 302 ( 0.00%)

Across different build trees:

 0$ export CCACHE_DIR=$(mktemp -d)
 1$ cmake -B build_1 -DWITH_CCACHE=ON
 2$ cmake --build build_1 -t bitcoind -j $(nproc)
 3$ cmake -B build_2 -DWITH_CCACHE=ON
 4$ ccache --zero-stats
 5$ cmake --build build_2 -t bitcoind -j $(nproc)
 6$ ccache --show-stats
 7Cacheable calls:    302 / 302 (100.0%)
 8  Hits:             302 / 302 (100.0%)
 9    Direct:         302 / 302 (100.0%)
10    Preprocessed:     0 / 302 ( 0.00%)
11  Misses:             0 / 302 ( 0.00%)
12Local storage:
13  Cache size (GiB): 0.2 / 5.0 ( 3.31%)
14  Hits:             302 / 302 (100.0%)
15  Misses:             0 / 302 ( 0.00%)

This PR addresses this comment:

However, ccache does not hit across two different build dirs, compiling the same commit.

Fixes #31771.

Form https://github.com/bitcoin/bitcoin/actions/runs/13215680798/job/36894673006:

0...
1Treat compiler warnings as errors ..... ON
2Use ccache for compiling .............. Built-in ccache support for the 'Visual Studio 17 2022' generator is not available.
3
4
5-- Configuring done (300.1s)
6-- Generating done (1.2s)
7-- Build files have been written to: D:/a/bitcoin/bitcoin/build

hebasto added the label Build system on Sep 10, 2024

DrahtBot commented at 11:56 am on September 10, 2024: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/30861.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	ryanofsky
Concept ACK	maflcko
Stale ACK	l0rinc, TheCharlatan, davidgumberg

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

No conflicts as of last run.

fanquake commented at 12:30 pm on September 10, 2024: member

100% cache hit rate is expected.

I tested this, and got about 90%:

 0ccache -C
 1ccache --zero-stats
 2
 3cmake -B some_build_dir -DWITH_CCACHE=ON
 4cmake --build some_build_dir -j17
 5
 6ccache --show-stats
 7Cacheable calls:    445 /  445 (100.0%)
 8  Hits:               0 /  445 ( 0.00%)
 9    Direct:           0
10    Preprocessed:     0
11  Misses:           445 /  445 (100.0%)
12Local storage:
13  Cache size (GiB): 0.2 / 30.0 ( 0.75%)
14  Hits:               0 /  445 ( 0.00%)
15  Misses:           445 /  445 (100.0%)
16
17ccache --zero-stats
18
19cmake -B /tmp/some_other_build_dir -DWITH_CCACHE=ON
20cmake --build /tmp/some_other_build_dir -j17
21
22ccache --show-stats
23Cacheable calls:    445 /  445 (100.0%)
24  Hits:             394 /  445 (88.54%)
25    Direct:         394 /  394 (100.0%)
26    Preprocessed:     0 /  394 ( 0.00%)
27  Misses:            51 /  445 (11.46%)
28Local storage:
29  Cache size (GiB): 0.2 / 30.0 ( 0.76%)
30  Hits:             394 /  445 (88.54%)
31  Misses:            51 /  445 (11.46%)

in cmake/ccache.cmake:5 in 66b73bbf37 outdated

1@@ -2,7 +2,7 @@
2 # Distributed under the MIT software license, see the accompanying
3 # file COPYING or https://opensource.org/license/mit/.
4 
5-if(NOT MSVC)
6+if(CMAKE_GENERATOR MATCHES "Ninja|Makefiles")

fanquake commented at 12:31 pm on September 10, 2024:

In 66b73bbf375ff96bf4a77539c9cf3fe52fbc15d5: can you explain the performance improvement (also how is it related to changing this conditional)?

hebasto commented at 12:43 pm on September 10, 2024:

When building across different build directories, using the base_dir option helps work around absolute paths in -I compiler options, increasing the ccache cache hit rate.

Maybe, the word “effectiveness” is more appropriate, rather than “performance”.

The change in the condition is not directly related to improving the hit rate. It enhances code correctness, as the method using of the CMAKE_{C,CXX}_COMPILER_LAUNCHER variables is applicable only to the mentioned generators.

ryanofsky commented at 2:07 pm on September 10, 2024:

In commit “build: Improve ccache performance for different build directories” (66b73bbf375ff96bf4a77539c9cf3fe52fbc15d5)

Would be good to add a comment explaining if(CMAKE_GENERATOR MATCHES "Ninja|Makefiles") saying that COMPILER_LAUNCHER variable only works for these backends not other backends. Otherwise the check seems arbitrary and doesn’t seem to make sense.

hebasto commented at 12:38 pm on September 12, 2024:

Would be good to add a comment explaining if(CMAKE_GENERATOR MATCHES "Ninja|Makefiles") saying that COMPILER_LAUNCHER variable only works for these backends not other backends. Otherwise the check seems arbitrary and doesn’t seem to make sense.

Thanks! Done.

in cmake/ccache.cmake:34 in b9fb3d1d99 outdated

23@@ -24,7 +24,10 @@ if(NOT MSVC)
24     set(WITH_CCACHE OFF)
25   endif()
26   if(WITH_CCACHE)
27-    try_append_cxx_flags("-fdebug-prefix-map=A=B" TARGET core_interface SKIP_LINK
28+    try_append_cxx_flags("-fdebug-prefix-map=A=B" SKIP_LINK
29+      TARGET core_interface
30+      # Propagate these flags, which apply to both C++ and C, to the secp256k1 subtree.

ryanofsky commented at 1:59 pm on September 10, 2024:

In commit “build: Propagate -fdebug-prefix-map flags to secp256k1 subtree” (b9fb3d1d99ae0b178b31c654b03a3b4f4a449fe7)

Is there some reason why only secp256k1 is specified here, not other subprojects like leveldb and crc32c? Would be good to expand comment to say whether secp256k1, or if this also makes sense to apply other projects.

hebasto commented at 2:31 pm on September 10, 2024:

For now, only the secp256k1 subtree uses its own build system. All other subtrees’ targets consume the core_interface, which propagates these flags to them.

ryanofsky commented at 2:42 pm on September 10, 2024:

re: #30861 (review)

For now, only the secp256k1 subtree uses its own build system. All other subtrees’ targets consume the core_interface, which propagates these flags to them.

Thanks! Might be good to add “because secp256k1, unlike other subprojects, does not use core_interface”

fanquake commented at 2:43 pm on September 10, 2024:

I don’t think “which apply to both C++ and C, " is relevant here. At least, mentioning C++ isn’t relevant for secp256k1.

theuni commented at 7:44 pm on September 18, 2024:

Arguably the issue here is that we have debug symbols for secp at all.

hebasto commented at 6:21 pm on November 5, 2024:

@theuni

Arguably the issue here is that we have debug symbols for secp at all.

What are your suggestions?

hebasto commented at 7:43 pm on January 16, 2025:

This is no longer relevant for the recent push.

in cmake/ccache.cmake:22 in 66b73bbf37 outdated

16@@ -17,8 +17,11 @@ if(NOT MSVC)
17       )
18       set(WITH_CCACHE ON)
19     elseif(WITH_CCACHE)
20-      list(APPEND CMAKE_C_COMPILER_LAUNCHER ${CCACHE_EXECUTABLE})
21-      list(APPEND CMAKE_CXX_COMPILER_LAUNCHER ${CCACHE_EXECUTABLE})
22+      foreach(lang IN ITEMS C CXX OBJCXX)
23+        set(CMAKE_${lang}_COMPILER_LAUNCHER
24+          ${CCACHE_EXECUTABLE} base_dir=${CMAKE_BINARY_DIR}

ryanofsky commented at 2:17 pm on September 10, 2024:

In commit “build: Improve ccache performance for different build directories” (66b73bbf375ff96bf4a77539c9cf3fe52fbc15d5)

Reading https://ccache.dev/manual/latest.html#_configuration_options it seems like it would make sense in most cases to set base_dir to CMAKE_SOURCE_DIR not CMAKE_BINARY_DIR.

For example if CMAKE_SOURCE_DIR is /home/bitcoin and CMAKE_BINARY_DIR is /home/bitcoin/build, it sounds like ccmake will not rewrite the include path -I/home/bitcoin/src to -I../src for greater cache reuse if base_dir=CMAKE_BINARY_DIR is used, because /home/bitcoin/src is not a subdirectory of /home/bitcoin/build

But it would rewrite the include path if CMAKE_SOURCE_DIR were used, because /home/bitcoin/src is a subdirectory of /home/bitcoin.

maflcko commented at 2:15 pm on September 11, 2024:

In commit “build: Improve ccache performance for different build directories” (https://github.com/bitcoin/bitcoin/commit/66b73bbf375ff96bf4a77539c9cf3fe52fbc15d5)

Now that base_dir is set by the build system, would it be fine to remove this from the prod notes?

0$ git grep --line-number base_dir doc/productivity.md 
1doc/productivity.md:42:base_dir = /home/yourname  # or wherever you keep your source files
2doc/productivity.md:45:Note: base_dir is required for ccache to share cached compiles of the same file across different repositories / paths; it will only do this for paths under base_dir. So this option is required for effective use of ccache with git worktrees (described below).
3doc/productivity.md:47:You _must not_ set base_dir to "/", or anywhere that contains system headers (according to the ccache docs).

hebasto commented at 3:22 pm on September 12, 2024:

CCACHE_BASEDIR=${CMAKE_BINARY_DIR} is effective when building in different build directories from the same source directory.

CCACHE_BASEDIR=${CMAKE_SOURCE_DIR} works for building from different source directories.

We need to choose one of these two options.

ryanofsky commented at 3:48 pm on September 12, 2024:

CCACHE_BASEDIR=${CMAKE_BINARY_DIR} is effective when building in different build directories from the same source directory.

CCACHE_BASEDIR=${CMAKE_SOURCE_DIR} works for building from different source directories.

We need to choose one of these two options.

Thanks, that clarifies a lot. I only do the second not the first, because I use git worktrees, and I assumed this PR was trying to make git worktrees and multiple checkouts work better. Maybe people who work on build systems are likely to have many different build directories inside the same source directory, but that would seem like an unusual thing for most other developers. Especially to have multiple build directories building with the same compilation options, because if compile options are different ccache command lines wouldn’t match anyway.

Is there a real use-case for having multiple different build directories with the same compilation options in the same source directory? And optimizing to have cache hits in that case?

Also maybe I need to think about it more, but I’m not sure why choosing CMAKE_BINARY_DIR would be better than CMAKE_SOURCE_DIR in either case as long a CMAKE_BINARY_DIR is a subdirectory of CMAKE_SOURCE_DIR. Are we trying to optimize for cases where CMAKE_BINARY_DIR is not a subdirectory?

hebasto commented at 4:13 pm on September 12, 2024:

Also maybe I need to think about it more, but I’m not sure why choosing CMAKE_BINARY_DIR would be better than CMAKE_SOURCE_DIR in either case as long a CMAKE_BINARY_DIR is a subdirectory of CMAKE_SOURCE_DIR. Are we trying to optimize for cases where CMAKE_BINARY_DIR is not a subdirectory?

I don’t think it is related to whether CMAKE_BINARY_DIR is a subdirectory of CMAKE_SOURCE_DIR or not.

Due to https://github.com/bitcoin/bitcoin/blob/07c7c96022dd325be1cd3b353d575eb6a5593f55/src/CMakeLists.txt#L9 all objects are compiled with the -I/absolute/path/to/build/dir and -I/absolute/path/to/source/dir flags, and any difference in either will trigger a cache miss.

The “Cache compilations with ccache” section of the Productivity Notes still applicable for this PR branch except for the last commit, which was mostly motivated by #30811 (comment).

ryanofsky commented at 4:21 pm on September 12, 2024:

I don’t think it is related to whether CMAKE_BINARY_DIR is a subdirectory of CMAKE_SOURCE_DIR or not.

Ccache documentation describes behavior of base_dir: “ccache will rewrite absolute paths into paths relative to the current working directory, but only absolute paths that begin with base_dir”

So if CMAKE_BINARY_DIR is a subdirectory of CMAKE_SOURCE_DIR, and base_dir is specified as CMAKE_SOURCE_DIR, then that should be a strict improvement because it will allow cache hits across multiple source directories, and multiple build directories, as long as build directories are not outside the source directories.

maflcko commented at 4:22 pm on September 12, 2024:

last commit, which was mostly motivated by #30811 (comment).

Sorry, I was just testing, not trying to imply that this is the common case. Using worktrees (different source dirs) seems more common than different build dirs for the same cmake config.

hebasto commented at 4:31 pm on September 12, 2024:

So if CMAKE_BINARY_DIR is a subdirectory of CMAKE_SOURCE_DIR, and base_dir is specified as CMAKE_SOURCE_DIR, then that should be a strict improvement because it will allow cache hits across multiple source directories, and multiple build directories, as long as build directories are not outside the source directories.

For the latter case, using different build directories will result in cache misses anyway, right?

Anyway, I agree with @maflcko and @ryanofsky that using git worktrees should be prioritised.

ryanofsky commented at 4:45 pm on September 12, 2024:

So if CMAKE_BINARY_DIR is a subdirectory of CMAKE_SOURCE_DIR, and base_dir is specified as CMAKE_SOURCE_DIR, then that should be a strict improvement because it will allow cache hits across multiple source directories, and multiple build directories, as long as build directories are not outside the source directories.

For the latter case, using different build directories will result in cache misses anyway, right?

It seems like that should be true but I haven’t experimented or thought about this much and am just going off of the documentation. If cmake adds include paths inside the build directory, maybe for generated headers, or for headers are copied or linked to from there, I could see imagine CMAKE_BINARY_DIR being better to use than CMAKE_SOURCE_DIR in some cases. But naively I would expect CMAKE_SOURCE_DIR being better in most cases, so that led me to ask the question.

hebasto commented at 4:37 pm on September 14, 2024:

@ryanofsky

I only do the second not the first, because I use git worktrees…

Thanks for your feedback. Your concerns should be addressed in the recent push.

hebasto commented at 7:39 pm on January 16, 2025:

In the recent push, this branch relies on setting the base_dir option by the user because the required value depends on actual user’s build environment.

hebasto commented at 7:42 pm on January 16, 2025:

Reworked to work equally well across both Git’s wortktrees and build trees.

ryanofsky approved

ryanofsky commented at 2:22 pm on September 10, 2024: contributor

Code review ACK 66b73bbf375ff96bf4a77539c9cf3fe52fbc15d5. This seems like a really helpful improvement, but did have some questions about it below.

in src/CMakeLists.txt:55 in 66b73bbf37 outdated

53@@ -54,13 +54,17 @@ include(GetTargetInterface)
54 # -fsanitize and related flags apply to both C++ and C,
55 # so we can pass them down to libsecp256k1 as CFLAGS and LDFLAGS.
56 get_target_interface(core_sanitizer_cxx_flags "" sanitize_interface COMPILE_OPTIONS)
57-set(SECP256K1_APPEND_CFLAGS ${core_sanitizer_cxx_flags} CACHE STRING "" FORCE)

fanquake commented at 2:47 pm on September 10, 2024:

In 21bfce58c1cad31801e045c606bd8fb16a00e11d: Just generally, I’d say it’s a bit less ideal, having secp256k1 flags/config getting split up into multiple (arbitrary) places. Especially if in future, we’ll have to duplicate all this logic in all places, for all subtrees (when using their own cmake build systems).

hebasto commented at 7:40 pm on January 16, 2025:

This is no longer relevant for the recent push.

DrahtBot added the label CI failed on Sep 10, 2024

DrahtBot commented at 11:35 pm on September 10, 2024: contributor

🚧 At least one of the CI tasks failed. Debug: https://github.com/bitcoin/bitcoin/runs/29930405099

Make sure to run all tests locally, according to the documentation.

The failure may happen due to a number of reasons, for example:

Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

hebasto commented at 3:49 pm on September 11, 2024: member

100% cache hit rate is expected.

I tested this, and got about 90%:

 0ccache -C
 1ccache --zero-stats
 2
 3cmake -B some_build_dir -DWITH_CCACHE=ON
 4cmake --build some_build_dir -j17
 5
 6ccache --show-stats
 7Cacheable calls:    445 /  445 (100.0%)
 8  Hits:               0 /  445 ( 0.00%)
 9    Direct:           0
10    Preprocessed:     0
11  Misses:           445 /  445 (100.0%)
12Local storage:
13  Cache size (GiB): 0.2 / 30.0 ( 0.75%)
14  Hits:               0 /  445 ( 0.00%)
15  Misses:           445 /  445 (100.0%)
16
17ccache --zero-stats
18
19cmake -B /tmp/some_other_build_dir -DWITH_CCACHE=ON
20cmake --build /tmp/some_other_build_dir -j17
21
22ccache --show-stats
23Cacheable calls:    445 /  445 (100.0%)
24  Hits:             394 /  445 (88.54%)
25    Direct:         394 /  394 (100.0%)
26    Preprocessed:     0 /  394 ( 0.00%)
27  Misses:            51 /  445 (11.46%)
28Local storage:
29  Cache size (GiB): 0.2 / 30.0 ( 0.76%)
30  Hits:             394 /  445 (88.54%)
31  Misses:            51 /  445 (11.46%)

On Ubuntu 24.04, I’ve got:

0$ ccache --show-stats 
1Cacheable calls:    456 / 456 (100.0%)
2  Hits:             456 / 456 (100.0%)
3    Direct:         456 / 456 (100.0%)
4    Preprocessed:     0 / 456 ( 0.00%)
5  Misses:             0 / 456 ( 0.00%)
6Local storage:
7  Cache size (GiB): 5.0 / 5.0 (100.0%)
8  Hits:             456 / 456 (100.0%)
9  Misses:             0 / 456 ( 0.00%)

for the build in the second directory. @fanquake What is your system and Ccache version?

fanquake commented at 4:07 pm on September 11, 2024: member

What is your system and Ccache version?

That was on a macOS machine, with 4.10.2. Retested just now, and got the same: Hits: 394 / 445 (88.54%).

fanquake commented at 4:16 pm on September 11, 2024: member

Just tested the same on a Fedora box (ccache 4.10.2) and ended up with Hits: 447 / 528 (84.66%).

hebasto commented at 4:20 pm on September 11, 2024: member

Just tested the same on a Fedora box (ccache 4.10.2) and ended up with Hits: 447 / 528 (84.66%).

What if you ccache --zero-stats after configuring for a new build tree, just before building?

On Fedora, ccache masquerades as a compiler, so all compilations during configuration can affect the final hit rate.

in cmake/module/TryAppendCXXFlags.cmake:45 in 2389371d35 outdated

42@@ -49,7 +43,7 @@ function(try_append_cxx_flags flags)
43     TACXXF                            # prefix
44     "SKIP_LINK"                       # options
45     "TARGET;VAR;SOURCE;RESULT_VAR"    # one_value_keywords

l0rinc commented at 8:29 pm on September 11, 2024:

It seemed to me that SOURCE is also unused here, see: 96cf00a (#30732)

hebasto commented at 7:36 pm on January 16, 2025:

This is no longer relevant for the recent push.

maflcko commented at 9:45 am on September 12, 2024: member

Needs rebase and CI failures fixed (they are true ones)

fanquake commented at 9:58 am on September 12, 2024: member

What if you ccache –zero-stats after configuring for a new build tree, just before building?

In that case on Fedora it is Hits: 446 / 446 (100.0%), with Uncacheable calls: 12 / 458 ( 2.62%).

hebasto force-pushed on Sep 12, 2024

hebasto marked this as a draft on Sep 12, 2024

hebasto force-pushed on Sep 12, 2024

hebasto commented at 2:45 pm on September 12, 2024: member

What if you ccache –zero-stats after configuring for a new build tree, just before building?

In that case on Fedora it is Hits: 446 / 446 (100.0%), with Uncacheable calls: 12 / 458 ( 2.62%).

If ccache --show-stats --verbose outputs:

0Uncacheable calls:      12 / 458 ( 2.62%)
1  Called for linking:   12 /  12 (100.0%)

this is expected on systems where ccache masquerades as a compiler.

DrahtBot removed the label CI failed on Sep 13, 2024

hebasto force-pushed on Sep 14, 2024

hebasto marked this as ready for review on Sep 14, 2024

hebasto commented at 4:35 pm on September 14, 2024: member

The PR has been reworked to keep compatibility with git worktrees. See this discussion.

The PR description has been updated accordingly.

maflcko added the label DrahtBot Guix build requested on Sep 16, 2024

fanquake commented at 8:43 am on September 16, 2024: member

Seems like you missed some review comments? https://github.com/bitcoin/bitcoin/pull/30861/files#r1752126150 #30861 (review)

Seems like #30861 (review) is now also more relevant given #30905, so it’d be good to know how this is going to be handed going forward.

DrahtBot commented at 8:57 pm on September 16, 2024: contributor

Guix builds (on x86_64) [untrusted test-only build, possibly unsafe, not for production use]

File	commit 0c4ff18ee9ec91b424ad26d2643e42566aa45e40(master)	commit 441c599254dcf81d7ca08f6509795041da1b6f03(master and this pull)
SHA256SUMS.part	`79b358d3fd281b65...`	`7480bb06dfe11941...`
*-aarch64-linux-gnu-debug.tar.gz	`4e496d0eb67133bf...`	`5b16ef1c658f30cc...`
*-aarch64-linux-gnu.tar.gz	`43490ff6ab9a74e2...`	`457bb82ec92bb12f...`
*-arm-linux-gnueabihf-debug.tar.gz	`38c68631f041e4d8...`	`6dd507cd8d0080f1...`
*-arm-linux-gnueabihf.tar.gz	`a80c39716b285df7...`	`2990906bac85028b...`
*-arm64-apple-darwin-unsigned.tar.gz	`59dce49b541dff6d...`	`eecbd5bd38a1a514...`
*-arm64-apple-darwin-unsigned.zip	`acb355306dcbd43c...`	`f58bd171b8bd2a6a...`
*-arm64-apple-darwin.tar.gz	`b6fa18bedb7fe497...`	`5013a4b3cb403494...`
*-powerpc64-linux-gnu-debug.tar.gz	`ab07199c08dcbf3d...`	`f13eb7b3069725d6...`
*-powerpc64-linux-gnu.tar.gz	`c93841f69a8a21f2...`	`bc628724a63587cd...`
*-riscv64-linux-gnu-debug.tar.gz	`fa11806d89e9abd7...`	`3fbcbb305f44259e...`
*-riscv64-linux-gnu.tar.gz	`379cad9361252115...`	`fb05d80a1c7a7f1b...`
*-x86_64-apple-darwin-unsigned.tar.gz	`03fd8b75278b79c7...`	`8c1dd1bdfbe1a736...`
*-x86_64-apple-darwin-unsigned.zip	`e7f5613fc474ffdb...`	`da01e15f8015fac3...`
*-x86_64-apple-darwin.tar.gz	`9c1621a91eba759d...`	`048e72e703a89270...`
*-x86_64-linux-gnu-debug.tar.gz	`0aaee8826bab2f2b...`	`fe32bc318f19c957...`
*-x86_64-linux-gnu.tar.gz	`163cb7ffd9eec265...`	`22f3e58d2456792d...`
*.tar.gz	`73ef39f7a7c8ccd9...`	`cfc14f97ef17ca32...`
guix_build.log	`2c42f4ee42e6a171...`	`2ce4aafa975c22ea...`
guix_build.log.diff		`0827acda3664ba74...`

DrahtBot removed the label DrahtBot Guix build requested on Sep 16, 2024

in cmake/ccache.cmake:24 in 3f8afcefb5 outdated

18@@ -17,15 +19,21 @@ if(NOT MSVC)
19       )
20       set(WITH_CCACHE ON)
21     elseif(WITH_CCACHE)
22-      list(APPEND CMAKE_C_COMPILER_LAUNCHER ${CCACHE_EXECUTABLE})
23-      list(APPEND CMAKE_CXX_COMPILER_LAUNCHER ${CCACHE_EXECUTABLE})
24+      foreach(lang IN ITEMS C CXX OBJCXX)
25+        set(CMAKE_${lang}_COMPILER_LAUNCHER
26+          ${CMAKE_COMMAND} -E env CCACHE_NOHASHDIR=1 ${CCACHE_EXECUTABLE}

l0rinc commented at 10:01 am on September 19, 2024:

Will this affect coverage or is that an old issue that doesn’t affect us anymore?

l0rinc commented at 10:05 am on September 19, 2024:

You can disable this option to get cache hits when compiling the same source code in different directories if you don’t mind that CWD in the debug info might be incorrect.

Does this affect us in any way?

l0rinc commented at 10:32 am on September 19, 2024:

Note that CCACHE_NOHASHDIR=1 was just removed in latest master’s CI config: https://github.com/bitcoin/bitcoin/pull/30869/files#diff-62587956f943bb2503db7bc6dd27d0d888074a1c0ecaab3f570ad611aff0f7bbL9

hebasto commented at 7:15 pm on January 16, 2025:

I did not test this branch for code coverage reports.

hebasto commented at 7:18 pm on January 16, 2025:

If it happens, the project might be re-built with -DWITH_CCACHE=OFF.

maflcko commented at 7:24 pm on January 16, 2025:

This should be harmless to set in the CI, because dir should be the same for all tasks anyway. If it isn’t then the compiler should differ as well.

stickies-v commented at 4:30 pm on January 31, 2025:

This does affect us wrt debug info. With this change, if you first compile in tree1, and then in tree2, inspecting the .o files in tree2 will reference the source files in tree1. I don’t know which, if any, use cases depend on that debug info being correct, but it’s potentially annoying at least?

Because env vars take precedence over config files, and because we’re appending this, I don’t think users would be able to undo this if they need correct debug info?

hebasto commented at 11:37 am on February 8, 2025:

This does affect us wrt debug info.

I don’t think it affects us too much because:https://github.com/bitcoin/bitcoin/blob/fb0ada982a73687520c43b8fde480fa5d456f3e1/CMakeLists.txt#L456-L458

l0rinc approved

l0rinc commented at 10:36 am on September 19, 2024: contributor

ACK 3f8afcefb53f18d735d9cd196df492d2d140c284

Normal build on master:

% ccache --clear --zero-stats % cmake -B build1 && cmake --build build1 -j10 % ccache --show-stats

0Cacheable calls:    440 / 440 (100.0%)
1  Hits:               0 / 440 ( 0.00%)
2    Direct:           0
3    Preprocessed:     0
4  Misses:           440 / 440 (100.0%)
5Local storage:
6  Cache size (GiB): 0.2 / 5.0 ( 4.89%)
7  Hits:               0 / 440 ( 0.00%)
8  Misses:           440 / 440 (100.0%)

% ccache --zero-stats % cmake -B build2 && cmake --build build2 -j10 % ccache --show-stats

0Cacheable calls:    440 / 440 (100.0%)
1  Hits:               0 / 440 ( 0.00%)
2    Direct:           0
3    Preprocessed:     0
4  Misses:           440 / 440 (100.0%)
5Local storage:
6  Cache size (GiB): 0.5 / 5.0 ( 9.78%)
7  Hits:               0 / 440 ( 0.00%)
8  Misses:           440 / 440 (100.0%)

Disabling `hash_dir` in ccache on `master`

% ccache --set-config hash_dir=false % ccache --zero-stats % cmake -B build4 && cmake --build build4 -j10 % ccache --show-stats

0bitcoin % ccache --show-stats
1Cacheable calls:    446 / 446 (100.0%)
2  Hits:             324 / 446 (72.65%)
3    Direct:          51 / 324 (15.74%)
4    Preprocessed:   273 / 324 (84.26%)
5  Misses:           122 / 446 (27.35%)
6Local storage:
7  Cache size (GiB): 1.1 / 5.0 (22.48%)
8  Hits:             324 / 446 (72.65%)
9  Misses:           122 / 446 (27.35%)

% ccache --set-config hash_dir=true

Normal build on rebased branch:

% ccache --clear --zero-stats % cmake -B build1 && cmake --build build1 -j10 % ccache --show-stats

0Cacheable calls:    446 / 446 (100.0%)
1  Hits:               0 / 446 ( 0.00%)
2    Direct:           0
3    Preprocessed:     0
4  Misses:           446 / 446 (100.0%)
5Local storage:
6  Cache size (GiB): 0.2 / 5.0 ( 4.97%)
7  Hits:               0 / 446 ( 0.00%)

% ccache --zero-stats % cmake -B build2 && cmake --build build2 -j10 % ccache --show-stats

0Cacheable calls:    446 / 446 (100.0%)
1  Hits:             324 / 446 (72.65%)
2    Direct:          51 / 324 (15.74%)
3    Preprocessed:   273 / 324 (84.26%)
4  Misses:           122 / 446 (27.35%)
5Local storage:
6  Cache size (GiB): 0.4 / 5.0 ( 7.74%)
7  Hits:             324 / 446 (72.65%)
8  Misses:           122 / 446 (27.35%)

Debug build (directly after a normal build)

% ccache --zero-stats % cmake -B build_debug1 -DCMAKE_BUILD_TYPE=Debug && cmake --build build_debug1 -j10 % ccache --show-stats

0Cacheable calls:    446 / 446 (100.0%)
1  Hits:               6 / 446 ( 1.35%)
2    Direct:           0 /   6 ( 0.00%)
3    Preprocessed:     6 /   6 (100.0%)
4  Misses:           440 / 446 (98.65%)
5Local storage:
6  Cache size (GiB): 0.6 / 5.0 (12.41%)
7  Hits:               6 / 446 ( 1.35%)
8  Misses:           440 / 446 (98.65%)

% ccache --zero-stats % cmake -B build_debug2 -DCMAKE_BUILD_TYPE=Debug && cmake --build build_debug2 -j10 % ccache --show-stats

0Cacheable calls:    446 / 446 (100.0%)
1  Hits:             324 / 446 (72.65%)
2    Direct:          51 / 324 (15.74%)
3    Preprocessed:   273 / 324 (84.26%)
4  Misses:           122 / 446 (27.35%)
5Local storage:
6  Cache size (GiB): 0.8 / 5.0 (15.02%)
7  Hits:             324 / 446 (72.65%)
8  Misses:           122 / 446 (27.35%)

DrahtBot requested review from ryanofsky on Sep 19, 2024

hebasto commented at 9:11 am on September 23, 2024: member

Seems like you missed some review comments? https://github.com/bitcoin/bitcoin/pull/30861/files#r1752126150 #30861 (comment)

Seems like #30861 (comment) is now also more relevant given #30905, so it’d be good to know how this is going to be handed going forward.

Converting to a draft for now.

hebasto marked this as a draft on Sep 23, 2024

maflcko commented at 2:03 pm on October 2, 2024: member

Concept ACK

DrahtBot added the label CI failed on Oct 22, 2024

DrahtBot removed the label CI failed on Oct 26, 2024

DrahtBot added the label Needs rebase on Nov 6, 2024

fanquake referenced this in commit eb2ebe6f30 on Dec 6, 2024

theuni commented at 7:18 pm on December 16, 2024: member

Needs rebase after #31231

fanquake commented at 12:06 pm on January 10, 2025: member

What is the status of this?

hebasto force-pushed on Jan 16, 2025

DrahtBot added the label CI failed on Jan 16, 2025

hebasto renamed this:
~~build: Improve `ccache` performance for different build directories~~
build: Enhance Ccache performance across worktrees and build trees
on Jan 16, 2025

hebasto marked this as ready for review on Jan 16, 2025

hebasto commented at 7:14 pm on January 16, 2025: member

What is the status of this?

Reworked. The PR description has been updated.

hebasto commented at 7:26 pm on January 16, 2025: member

The CI failure is unrelated.

hebasto removed the label Needs rebase on Jan 17, 2025

DrahtBot removed the label CI failed on Jan 17, 2025

fanquake added this to the milestone 29.0 on Jan 21, 2025

davidgumberg commented at 0:59 am on January 29, 2025: contributor

Tested ACK https://github.com/bitcoin/bitcoin/commit/4d3f360e2af94f4ed46dc0943196d548da82003e

Fixes worktree and build tree ccache hits.

Note that as the PR description states, in the latest push, setting a BASE_DIR for ccache (as advised in the productivity notes) is essential for ccache to get hits in different directories.

All on fresh ubuntu 22.04.1 vm images running running with qemu, in variations I’ve omitted the three setup commands installing deps and git cloning.

 0# setup, ommitted in later examples
 1sudo apt upgrade -y
 2sudo apt install build-essential ccache clang cmake git libboost-dev libevent-dev libsqlite3-dev pkgconf -y
 3git clone https://github.com/bitcoin/bitcoin
 4
 5
 6export CC=clang CXX=clang++
 7
 8mkdir -p ~/.config/ccache
 9echo  "base_dir = ${HOME}" > ~/.config/ccache/ccache.conf
10cd bitcoin
11git fetch origin pull/30861/head:30861 && git switch 30861
12cmake -B build && cmake --build build -j $(nproc)
13git worktree add ../other && cd ../other
14cmake -B build && cmake --build build -j $(nproc)

 0$ ccache --show-stats --verbose
 1Cacheable calls:     924 / 924 (100.0%)
 2  Hits:              462 / 924 (50.00%)
 3    Direct:          462 / 462 (100.0%)
 4    Preprocessed:      0 / 462 ( 0.00%)
 5  Misses:            462 / 924 (50.00%)
 6Successful lookups:
 7  Direct:            462 / 924 (50.00%)
 8  Preprocessed:        0 / 462 ( 0.00%)
 9Local storage:
10  Cache size (GiB):  0.1 / 5.0 ( 2.42%)
11  Files:             924
12  Hits:              462 / 924 (50.00%)
13  Misses:            462 / 924 (50.00%)
14  Reads:            1848
15  Writes:            924

And buildtrees also work:

0mkdir -p ~/.config/ccache
1echo  "base_dir = ${HOME}" > ~/.config/ccache/ccache.conf
2cd bitcoin
3git fetch origin pull/30861/head:30861 && git switch 30861
4cmake -B build && cmake --build build -j $(nproc)
5cmake -B build2 && cmake --build build2 -j $(nproc)

 0$ ccache --show-stats --verbose
 1Cacheable calls:     924 / 924 (100.0%)
 2  Hits:              462 / 924 (50.00%)
 3    Direct:          462 / 462 (100.0%)
 4    Preprocessed:      0 / 462 ( 0.00%)
 5  Misses:            462 / 924 (50.00%)
 6Successful lookups:
 7  Direct:            462 / 924 (50.00%)
 8  Preprocessed:        0 / 462 ( 0.00%)
 9Local storage:
10  Cache size (GiB):  0.1 / 5.0 ( 2.42%)
11  Files:             924
12  Hits:              462 / 924 (50.00%)
13  Misses:            462 / 924 (50.00%)
14  Reads:            1848
15  Writes:            924

No basedir, pr branch, clang

 0export CC=clang CXX=clang++
 1
 2# don't set a base dir in this run
 3# mkdir -p ~/.config/ccache
 4# echo  "base_dir = ${HOME}" > ~/.config/ccache/ccache.conf
 5cd bitcoin
 6
 7git fetch origin pull/30861/head:30861 && git switch 30861
 8cmake -B build && cmake --build build -j $(nproc)
 9git worktree add ../other && cd ../other
10cmake -B build && cmake --build build -j $(nproc)

 0$ ccache --show-stats --verbose
 1Cacheable calls:     924 / 924 (100.0%)
 2  Hits:                0 / 924 ( 0.00%)
 3    Direct:            0
 4    Preprocessed:      0
 5  Misses:            924 / 924 (100.0%)
 6Successful lookups:
 7  Direct:              0 / 924 ( 0.00%)
 8  Preprocessed:        0 / 924 ( 0.00%)
 9Local storage:
10  Cache size (GiB):  0.2 / 5.0 ( 4.83%)
11  Files:            1848
12  Hits:                0 / 924 ( 0.00%)
13  Misses:            924 / 924 (100.0%)
14  Reads:            1848
15  Writes:           1848

basedir set, master, clang

0export CC=clang CXX=clang++
1
2mkdir -p ~/.config/ccache
3echo  "base_dir = ${HOME}" > ~/.config/ccache/ccache.conf
4cd bitcoin
5
6# git fetch origin pull/30861/head:30861 && git switch 30861
7cmake -B build && cmake --build build -j $(nproc)
8git worktree add ../other && cd ../other
9cmake -B build && cmake --build build -j $(nproc)

 0$ ccache --show-stats --verbose
 1Stats updated:      Tue Jan 28 23:44:43 2025
 2Cacheable calls:     924 / 924 (100.0%)
 3  Hits:                0 / 924 ( 0.00%)
 4    Direct:            0
 5    Preprocessed:      0
 6  Misses:            924 / 924 (100.0%)
 7Successful lookups:
 8  Direct:              0 / 924 ( 0.00%)
 9  Preprocessed:        0 / 924 ( 0.00%)
10Local storage:
11  Cache size (GiB):  0.2 / 5.0 ( 4.84%)
12  Files:            1848
13  Hits:                0 / 924 ( 0.00%)
14  Misses:            924 / 924 (100.0%)
15  Reads:            1848
16  Writes:           1848

basedir set, pr branch, gcc

0export CC=gcc CXX=g++
1
2mkdir -p ~/.config/ccache
3echo  "base_dir = ${HOME}" > ~/.config/ccache/ccache.conf
4cd bitcoin
5git fetch origin pull/30861/head:30861 && git switch 30861
6cmake -B build && cmake --build build -j $(nproc)
7git worktree add ../other && cd ../other
8cmake -B build && cmake --build build -j $(nproc)

 0$ ccache --show-stats --verbose
 1Cacheable calls:     924 / 924 (100.0%)
 2  Hits:              462 / 924 (50.00%)
 3    Direct:          462 / 462 (100.0%)
 4    Preprocessed:      0 / 462 ( 0.00%)
 5  Misses:            462 / 924 (50.00%)
 6Successful lookups:
 7  Direct:            462 / 924 (50.00%)
 8  Preprocessed:        0 / 462 ( 0.00%)
 9Local storage:
10  Cache size (GiB):  0.3 / 5.0 ( 6.55%)
11  Files:             924
12  Hits:              462 / 924 (50.00%)
13  Misses:            462 / 924 (50.00%)
14  Reads:            1848
15  Writes:            924

basedir set, master, gcc

0export CC=gcc CXX=g++
1
2mkdir -p ~/.config/ccache
3echo  "base_dir = ${HOME}" > ~/.config/ccache/ccache.conf
4cd bitcoin
5# git fetch origin pull/30861/head:30861 && git switch 30861
6cmake -B build && cmake --build build -j $(nproc)
7git worktree add ../other && cd ../other
8cmake -B build && cmake --build build -j $(nproc)

 0$ ccache --show-stats --verbose
 1Cacheable calls:     924 / 924 (100.0%)
 2  Hits:              462 / 924 (50.00%)
 3    Direct:          462 / 462 (100.0%)
 4    Preprocessed:      0 / 462 ( 0.00%)
 5  Misses:            462 / 924 (50.00%)
 6Successful lookups:
 7  Direct:            462 / 924 (50.00%)
 8  Preprocessed:        0 / 462 ( 0.00%)
 9Local storage:
10  Cache size (GiB):  0.3 / 5.0 ( 6.55%)
11  Files:             924
12  Hits:              462 / 924 (50.00%)
13  Misses:            462 / 924 (50.00%)
14  Reads:            1848
15  Writes:            924

No basedir, master, clang

 0export CC=clang CXX=clang++
 1
 2# don't set a base dir in this run
 3# mkdir -p ~/.config/ccache
 4# echo  "base_dir = ${HOME}" > ~/.config/ccache/ccache.conf
 5cd bitcoin
 6
 7# git fetch origin pull/30861/head:30861 && git switch 30861
 8cmake -B build && cmake --build build -j $(nproc)
 9git worktree add ../other && cd ../other
10cmake -B build && cmake --build build -j $(nproc)

 0$ ccache --show-stats --verbose
 1Cacheable calls:     924 / 924 (100.0%)
 2  Hits:                0 / 924 ( 0.00%)
 3    Direct:            0
 4    Preprocessed:      0
 5  Misses:            924 / 924 (100.0%)
 6Successful lookups:
 7  Direct:              0 / 924 ( 0.00%)
 8  Preprocessed:        0 / 924 ( 0.00%)
 9Local storage:
10  Cache size (GiB):  0.7 / 5.0 (13.14%)
11  Files:            1848
12  Hits:                0 / 924 ( 0.00%)
13  Misses:            924 / 924 (100.0%)
14  Reads:            1848
15  Writes:           1848

DrahtBot requested review from maflcko on Jan 29, 2025

DrahtBot requested review from l0rinc on Jan 29, 2025

in cmake/ccache.cmake:23 in 4d3f360e2a outdated

18@@ -17,8 +19,11 @@ if(NOT MSVC)
19       )
20       set(WITH_CCACHE ON)
21     elseif(WITH_CCACHE)
22-      list(APPEND CMAKE_C_COMPILER_LAUNCHER ${CCACHE_EXECUTABLE})
23-      list(APPEND CMAKE_CXX_COMPILER_LAUNCHER ${CCACHE_EXECUTABLE})
24+      foreach(lang IN ITEMS C CXX OBJCXX)
25+        set(CMAKE_${lang}_COMPILER_LAUNCHER

TheCharlatan commented at 2:10 pm on January 30, 2025:

Why is this now overriding the variable instead of appending?

hebasto commented at 3:29 pm on January 30, 2025:

Thanks!

I do not really have an explanation for this change…

We should definitely honour user-provided CMAKE_<LANG>_COMPILER_LAUNCHER environment variables, if any.

The appending has been restored.

hebasto commented at 12:40 pm on February 8, 2025:

Reverted back given this comment.

hebasto force-pushed on Jan 30, 2025

TheCharlatan approved

TheCharlatan commented at 4:16 pm on January 30, 2025: contributor

ACK aeb3977db51792d8ad22b0a5ff8fc5ff20d69a00

DrahtBot requested review from davidgumberg on Jan 30, 2025

davidgumberg commented at 5:01 pm on January 30, 2025: contributor

utreACK https://github.com/bitcoin/bitcoin/commit/aeb3977db51792d8ad22b0a5ff8fc5ff20d69a00

in cmake/ccache.cmake:6 in aeb3977db5 outdated

1@@ -2,7 +2,9 @@
2 # Distributed under the MIT software license, see the accompanying
3 # file COPYING or https://opensource.org/license/mit/.
4 
5-if(NOT MSVC)
6+# The <LANG>_COMPILER_LAUNCHER target property, used to integrate
7+# Ccache, is supported only by the Makefiles and Ninja generators.

stickies-v commented at 11:43 am on January 31, 2025:

nit

0# Ccache, is supported only by the Makefile and Ninja generators.

hebasto commented at 12:39 pm on February 8, 2025:

Thanks! Fixed.

in cmake/ccache.cmake:7 in aeb3977db5 outdated

1@@ -2,7 +2,9 @@
2 # Distributed under the MIT software license, see the accompanying
3 # file COPYING or https://opensource.org/license/mit/.
4 
5-if(NOT MSVC)
6+# The <LANG>_COMPILER_LAUNCHER target property, used to integrate
7+# Ccache, is supported only by the Makefiles and Ninja generators.
8+if(CMAKE_GENERATOR MATCHES "Make|Ninja")

stickies-v commented at 12:47 pm on January 31, 2025:

I think we could drop this check entirely? As per <LANG>_COMPILER_LAUNCHER:

The Makefile Generators and the Ninja generator will run this tool and pass the compiler and its arguments to the tool.

If the generator picks it up (i.e. currently Makefile, Ninja), great. If it doesn’t (i.e. MSVC), fine (I think?). If in the future it changes (i.e. MSVC picks it up, or Makefile drops it), also great. Hardcoding this dependency seems unnecessary to me, and not consistent either because the user will still see WITH_CCACHE=ON for these unsupported generators? I don’t think it’s our responsibility to guarantee that ccache will work when the user requests it?

Possible alternatives would be:

remove the generator check and document -DWITH_CCACHE’s dependency on the generator, although I think the current documentation (Attempt to use ccache for compiling.) is sufficient.
keep the hardcoded dependency and toggle WITH_CCACHE off if not Ninja or Makefile

willcl-ark commented at 2:30 pm on February 4, 2025:

We could also (or additionally) check if the user has set the launcher themselves by gating the entire following section with:

0 if(NOT DEFINED CMAKE_CXX_COMPILER_LAUNCHER)

…skipping all ccache setup if they have.

This way we might be able to get the best of both “sane defaults” and “easy to override”.

hebasto commented at 12:39 pm on February 8, 2025:

Thanks! Reworked per your feedback.

ryanofsky commented at 4:50 pm on February 20, 2025:

re: #30861 (review)

In commit “cmake: Clarify ccache support for different generators” (c19a187c42fe867d61ca5dbd48ae18f15201839f)

Thanks! Reworked per your feedback.

It is hard to tell from reading this thread if the “I think we could drop this check entirely” suggestion and other suggestions have been addressed or not. The check appears to be unchanged. Would be good to clear this up.

in cmake/ccache.cmake:23 in aeb3977db5 outdated

18@@ -17,8 +19,9 @@ if(NOT MSVC)
19       )
20       set(WITH_CCACHE ON)
21     elseif(WITH_CCACHE)
22-      list(APPEND CMAKE_C_COMPILER_LAUNCHER ${CCACHE_EXECUTABLE})
23-      list(APPEND CMAKE_CXX_COMPILER_LAUNCHER ${CCACHE_EXECUTABLE})
24+      foreach(lang IN ITEMS C CXX OBJCXX)
25+        list(APPEND CMAKE_${lang}_COMPILER_LAUNCHER ${CMAKE_COMMAND} -E env CCACHE_NOHASHDIR=1 ${CCACHE_EXECUTABLE})

stickies-v commented at 4:35 pm on January 31, 2025:

Perhaps this is obvious in CMake land, but I was confused by what ${CMAKE_COMMAND} -E env is doing. I would have appreciated a docstring like the below:

    # Use `cmake -E env` as a cross-platform way to set the CCACHE_NOHASHDIR environment variable, increasing
    # ccache hit rate e.g. in case of multiple build directories or git worktrees.

hebasto commented at 12:38 pm on February 8, 2025:

Thanks! The comment has been added.

stickies-v commented at 4:41 pm on January 31, 2025: contributor

Definitely in favour of increasing ccache effectiveness, but I’m still figuring out if this approach is optimal. Is my understanding correct that with this PR, all we’re doing is forcing the CCACHE_NOHASHDIR=1 environment variable, instead of letting the user set it?

While not unreasonable, it does seem to make it difficult for the user to undo this (without disabling ccache entirely, which also isn’t ideal), and I’m not sure if this wouldn’t break any workflows (see #30861 (review))? If my understanding is correct, I think a viable alternative would be to mention this environment variable in our productivity notes?

cmake: Clarify ccache support for different generators

To enhance the user experience when using ccache, we utilize the CMake's
`<LANG>_COMPILER_LAUNCHER` target properties, which are only supported
by the Makefile and Ninja generators.

c19a187c42

cmake: Enhance ccache performance across worktrees and build directories

To maximize performance, it is essential to follow the recommendations
in the Productivity Notes: set the `base_dir` ccache configuration
option or use the `CCACHE_BASEDIR` environment variable.

82a9f24f49

hebasto force-pushed on Feb 8, 2025

hebasto commented at 12:37 pm on February 8, 2025: member

Thanks to all for the review!

Your feedback has been addressed.

Additionally, #31771 has been fixed.

in cmake/ccache.cmake:26 in 82a9f24f49

23-      list(APPEND CMAKE_CXX_COMPILER_LAUNCHER ${CCACHE_EXECUTABLE})
24+      foreach(lang IN ITEMS C CXX OBJCXX)
25+        if(DEFINED CMAKE_${lang}_COMPILER_LAUNCHER)
26+          list(APPEND configure_warnings
27+            "CMAKE_${lang}_COMPILER_LAUNCHER is already set. Skipping built-in ccache support for ${lang}."
28+          )

davidgumberg commented at 5:41 pm on February 13, 2025:

Is making this a configure_warning compatible with #31665? Maybe it would be a benefit if CI complained when unintentionally skipping ccache?

hebasto commented at 8:46 pm on February 13, 2025:

Would you mind elaborating on your suggestion?

davidgumberg commented at 9:47 pm on February 13, 2025:

Maybe better expressed as: Is trying to both override the compiler launcher and make configure_warnings errors a use case that should be supported? If so, making this message a configure_warning is incompatible with #31665 since that makes configure_warnings fatal errors if WCONFIGURE_ERROR=ON.

If it is not a use case that should be supported, then making this message a configure_warning might be good together with #31665, since it would cause an error in CI if ccache ever got disabled because a compiler launcher was set, but maybe there isn’t a realistic situation where something like that could happen unintentionally.

davidgumberg commented at 8:32 pm on February 14, 2025:

It’s also fair if you think #31665 is a bad approach and this is more evidence of that.

ryanofsky approved

ryanofsky commented at 5:32 pm on February 20, 2025: contributor

Code review ACK 82a9f24f49b30c9462716cf0cbf207499be13edf since this seems like a reasonable change, but I’m confused about why it actually doing anything and agree with stickies concern #30861#pullrequestreview-2586373527 that forcing CCACHE_NOHASHDIR=1 might not be ideal, especially because according to documentation for hash_dir it sounds like if CWD is not hashed and -fdebug-prefix-map is not used, objects could be built with the wrong debug information.

But mainly I’m just confused why this change has any effect at all given that hash_dir documentation says “The CWD will not be included in the hash if base_dir is set (and matches the CWD) and the compiler option -fdebug-prefix-map is used.” It seems like we are doing (or requiring) both of these things so I don’t see why the hash_dir setting should change anything. The results posted #30861 (comment) do seem to show it having an effect though, so think I must be misunderstanding something.

Also, I’m confused by why we need to require manually setting base_dir instead of setting it automatically to CMAKE_SOURCE_DIR. I don’t understand comment #30861 (review) about why this would fail to cache multiple build directories, since most of the time build directories will be under the source directory.

DrahtBot requested review from TheCharlatan on Feb 20, 2025

DrahtBot requested review from davidgumberg on Feb 20, 2025

hebasto commented at 5:41 pm on February 20, 2025: member

Also, I’m confused by why we need to require manually setting base_dir instead of setting it automatically to CMAKE_SOURCE_DIR.

Setting base_dir to CMAKE_SOURCE_DIR won’t be helpful in cases when a worktree or a build directory resides outside of the repository root, for example, it is a sibling directory.

ryanofsky commented at 6:38 pm on February 20, 2025: contributor

Setting base_dir to CMAKE_SOURCE_DIR won’t be helpful in cases when a worktree or a build directory resides outside of the repository root, for example, it is a sibling directory.

I don’t understand why it wouldn’t be helpful when a worktree is outside the repository root. If source directories are /tree1/ and /tree2 and build directories are /tree1/build and /tree2/build I would expect source paths like src/init.cpp to be treated like ../src/init.cpp in both cases and for the caching to work. Would also expect this to generalize regardless of how the build directories are named as long as they are at the same level relative to the source directory. But, very possible I’m missing something since I haven’t tested this.

I do understand that setting base_dir to CMAKE_SOURCE_DIR will not work if developers are placing their build directories completely outside of their source directories, or at inconsistent depths within their source directories, but this seems much less common and not really a consideration if we are just deciding on a default base_dir value.

Separately, I’m still confused about why setting hash_dir has an effect (mentioned #30861#pullrequestreview-2630570206) so there are definitely things I’m not grasping here, but in any case this change seems at least safe and I am happy if it works in practice.

hebasto commented at 6:45 pm on February 20, 2025: member

I do understand that setting base_dir to CMAKE_SOURCE_DIR will not work if developers are placing their build directories completely outside of their source directories, or at inconsistent depths within their source directories, but this seems much less common and not really a consideration if we are just deciding on a default base_dir value.

Right. As for the PR author, it was not clear to me which cases are more common and which are less common, so I tried to take a more general approach.

Ultimately, the mentioning of base_dir might be dropped, leaving it up to the user.

hebasto commented at 9:48 am on February 21, 2025: member

… I’m confused about why it actually doing anything and agree with stickies concern #30861 (review) that forcing CCACHE_NOHASHDIR=1 might not be ideal, especially because according to documentation for hash_dir it sounds like if CWD is not hashed and -fdebug-prefix-map is not used, objects could be built with the wrong debug information.

I’m wondering in which scenarios a user might wish to disable the -fdebug-prefix-map flag?

But mainly I’m just confused why this change has any effect at all given that hash_dir documentation says “The CWD will not be included in the hash if base_dir is set (and matches the CWD) and the compiler option -fdebug-prefix-map is used.” It seems like we are doing (or requiring) both of these things so I don’t see why the hash_dir setting should change anything. The results posted #30861 (comment) do seem to show it having an effect though, so think I must be misunderstanding something.

For the CWD to be omitted from hashing when a base_dir is set, it must match the base_dir.

build: Enhance Ccache performance across worktrees and build trees #30861

Code Coverage & Benchmarks

Reviews

Conflicts

Guix builds (on x86_64) [untrusted test-only build, possibly unsafe, not for production use]

Normal build on master:

Disabling hash_dir in ccache on master

Normal build on rebased branch:

Debug build (directly after a normal build)

And buildtrees also work:

No basedir, pr branch, clang

basedir set, master, clang

basedir set, pr branch, gcc

basedir set, master, gcc

No basedir, master, clang

Disabling `hash_dir` in ccache on `master`