build: Improve `ccache` performance for different build directories

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

This PR makes it possible to reuse the ccache cache populated during a build in another build directory.

For example, on Ubuntu 24.04:

 0$ cmake -B build_1 -DWITH_CCACHE=ON
 1$ cmake --build build_1 -t bitcoind
 2$ cmake -B build_2 -DWITH_CCACHE=ON
 3$ ccache --zero-stats
 4$ cmake --build build_2 -t bitcoind
 5$ ccache --show-stats
 6Cacheable calls:    298 / 298 (100.0%)
 7  Hits:             221 / 298 (74.16%)
 8    Direct:          59 / 221 (26.70%)
 9    Preprocessed:   162 / 221 (73.30%)
10  Misses:            77 / 298 (25.84%)
11<snip>

Further improvements can be made by replacing the config/bitcoin-config.h header with -D preprocessor options in the command line, eliminating the need to specifying -I${CMAKE_CURRENT_BINARY_DIR}.

The “Cache compilations with ccache” section of the Productivity Notes still applicable.

Please refer to #20353 for historical context.

Addresses this comment:

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

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	l0rinc
Concept ACK	maflcko
Stale ACK	ryanofsky

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

#31053 (RFC: build: support for pre-compiled headers. by theuni)
#30811 (build: Unify -logsourcelocations format by hebasto)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

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?

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.

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:62 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).

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)

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

build: Fix `IF_CHECK_PASSED` option implementation

Since `IF_CHECK_PASSED` is a multi_value_keyword, its value is a list.
It must be converted to a string before applying any `string()` command.

0d47247234

build: Prevent build path from leaking into debug info

This change improves compatibility with `ccache` when building in
different directories.

4d39cc8a48

build, refactor: Allow SECP256K1_APPEND_{C,LD}FLAGS to be set elsewhere e926af72f4

build: Propagate `-fdebug-prefix-map` flags to secp256k1 subtree

This change improves compatibility with `ccache` for object files being
built in the secp256k1 subtree.

416e13317d

build: Improve `ccache` performance for different build directories 3f8afcefb5

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

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

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

DrahtBot commented at 11:40 am on November 6, 2024: contributor

🐙 This pull request conflicts with the target branch and needs rebase.

build: Improve ccache performance for different build directories #30861