Skip to content

Commit ede388d

Browse files
committed
Merge bitcoin/bitcoin#30911: build: simplify by flattening the dependency graph
12fa951 build: simplify dependency graph (Cory Fields) c4e4983 build: avoid unnecessary dependencies on generated headers (Cory Fields) Pull request description: These changes speed up my build (default config/options/targets) by roughly 10%. I suspect the difference may be more significant in other build configs. Before: > $ time cmake --build build -j24 > real3m26.932s After: > $ time cmake --build build -j24 > real3m7.556s Generally they allow for jobservers (either `make -jX` or `ninja`) to be better utilized. This can be verified using `top` while building and looking at the number of compiles running at any given time before/after these changes. Before, it's easy to observe periods of stalling when only one or two compiles are happening. After these changes, the compiler process count should mostly match the number of jobs given (`-jX`) until it falls off at the end. --- The first commit sets [DEPENDS_EXPLICIT_ONLY](https://cmake.org/cmake/help/latest/command/add_custom_command.html#command:add_custom_command) for commands which generate our test header files. Without this option, `test_bitcoin`'s generated headers won't be built until all of its other dependencies have been built. This introduces a significant stall in the build, though currently only Ninja benefits from this being set, and only CMake >= 3.27 understands it. Example from a generated `build.ninja`: Before: > \# Custom command for src/test/data/base58_encode_decode.json.h > > build src/test/data/base58_encode_decode.json.h | ${cmake_ninja_workdir}src/test/data/base58_encode_decode.json.h: CUSTOM_COMMAND /home/cory/dev/bitcoin/src/test/data/base58_encode_decode.json /home/cory/dev/bitcoin/cmake/script/GenerateHeaderFromJson.cmake || libcrc32c.a libcrc32c_sse42.a libleveldb.a libminisketch.a minisketch_clmul src/bitcoin_clientversion src/crypto/libbitcoin_crypto.a src/crypto/libbitcoin_crypto_avx2.a src/crypto/libbitcoin_crypto_sse41.a src/crypto/libbitcoin_crypto_x86_shani.a src/generate_build_info src/libbitcoin_cli.a src/libbitcoin_common.a src/libbitcoin_consensus.a src/libbitcoin_node.a src/secp256k1/src/libsecp256k1.a src/secp256k1/src/secp256k1_precomputed src/test/util/libtest_util.a src/univalue/libunivalue.a src/util/libbitcoin_util.a src/wallet/libbitcoin_wallet.a src/zmq/libbitcoin_zmq.a After: > \# Custom command for src/test/data/base58_encode_decode.json.h > > build src/test/data/base58_encode_decode.json.h | ${cmake_ninja_workdir}src/test/data/base58_encode_decode.json.h: CUSTOM_COMMAND /home/cory/dev/bitcoin/src/test/data/base58_encode_decode.json /home/cory/dev/bitcoin/cmake/script/GenerateHeaderFromJson.cmake --- The second commit is more significant. It sets [CMAKE_OPTIMIZE_DEPENDENCIES](https://cmake.org/cmake/help/latest/prop_tgt/OPTIMIZE_DEPENDENCIES.html) globally, which allows the objects of static libs to be built in parallel when one lib depends on the other. This can be set as a per-lib property, ~but I don't see any need for that as we don't currently have any edge-cases where this wouldn't be ok. If those should arise, we could always disable on a per-lib basis~. Edit: turns out this triggers an [upstream bug](https://gitlab.kitware.com/cmake/cmake/-/issues/24058), which I guess can be considered an edge-case until fixed in CMake. I've added 2 per-lib opt-outs as a result. Example: Before: > \# Link the static library src/libbitcoin_cli.a > > build src/libbitcoin_cli.a: CXX_STATIC_LIBRARY_LINKER__bitcoin_cli_RelWithDebInfo src/CMakeFiles/bitcoin_cli.dir/compat/stdin.cpp.o src/CMakeFiles/bitcoin_cli.dir/rpc/client.cpp.o || src/univalue/libunivalue.a After: > \# Link the static library src/libbitcoin_cli.a > > build src/libbitcoin_cli.a: CXX_STATIC_LIBRARY_LINKER__bitcoin_cli_RelWithDebInfo src/CMakeFiles/bitcoin_cli.dir/compat/stdin.cpp.o src/CMakeFiles/bitcoin_cli.dir/rpc/client.cpp.o > ACKs for top commit: l0rinc: utACK 12fa951 hebasto: ACK 12fa951. Tree-SHA512: f85f507e70cdc06acd07542161d9f9b8edf9ba866f08c8ef17aaaed770fa11530a27521c4413456d863463a6e77d4d6983fa623a64e17bbd602c2bc70aacc112
2 parents 534414c + 12fa951 commit ede388d

File tree

4 files changed

+21
-0
lines changed

4 files changed

+21
-0
lines changed

CMakeLists.txt

+7
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,13 @@ set(CMAKE_CXX_EXTENSIONS OFF)
7272
list(APPEND CMAKE_MODULE_PATH ${PROJECT_SOURCE_DIR}/cmake/module)
7373
include(ProcessConfigurations)
7474

75+
# Flatten static lib dependencies.
76+
# Without this, if libfoo.a depends on libbar.a, libfoo's objects can't begin
77+
# to be compiled until libbar.a has been created.
78+
if (NOT DEFINED CMAKE_OPTIMIZE_DEPENDENCIES)
79+
set(CMAKE_OPTIMIZE_DEPENDENCIES TRUE)
80+
endif()
81+
7582
#=============================
7683
# Configurable options
7784
#=============================

cmake/minisketch.cmake

+3
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,9 @@ add_library(minisketch STATIC EXCLUDE_FROM_ALL
7474
${PROJECT_SOURCE_DIR}/src/minisketch/src/fields/generic_8bytes.cpp
7575
)
7676

77+
# Workaround for https://gitlab.kitware.com/cmake/cmake/-/issues/24058
78+
set_target_properties(minisketch PROPERTIES OPTIMIZE_DEPENDENCIES OFF)
79+
7780
target_include_directories(minisketch
7881
PUBLIC
7982
$<BUILD_INTERFACE:${PROJECT_SOURCE_DIR}/src/minisketch/include>

cmake/module/GenerateHeaders.cmake

+8
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,19 @@
22
# Distributed under the MIT software license, see the accompanying
33
# file COPYING or https://opensource.org/license/mit/.
44

5+
if(CMAKE_VERSION VERSION_GREATER_EQUAL 3.27)
6+
set(DEPENDS_EXPLICIT_OPT DEPENDS_EXPLICIT_ONLY)
7+
else()
8+
set(DEPENDS_EXPLICIT_OPT)
9+
endif()
10+
511
function(generate_header_from_json json_source_relpath)
612
add_custom_command(
713
OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/${json_source_relpath}.h
814
COMMAND ${CMAKE_COMMAND} -DJSON_SOURCE_PATH=${CMAKE_CURRENT_SOURCE_DIR}/${json_source_relpath} -DHEADER_PATH=${CMAKE_CURRENT_BINARY_DIR}/${json_source_relpath}.h -P ${PROJECT_SOURCE_DIR}/cmake/script/GenerateHeaderFromJson.cmake
915
DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/${json_source_relpath} ${PROJECT_SOURCE_DIR}/cmake/script/GenerateHeaderFromJson.cmake
1016
VERBATIM
17+
${DEPENDS_EXPLICIT_OPT}
1118
)
1219
endfunction()
1320

@@ -17,5 +24,6 @@ function(generate_header_from_raw raw_source_relpath raw_namespace)
1724
COMMAND ${CMAKE_COMMAND} -DRAW_SOURCE_PATH=${CMAKE_CURRENT_SOURCE_DIR}/${raw_source_relpath} -DHEADER_PATH=${CMAKE_CURRENT_BINARY_DIR}/${raw_source_relpath}.h -DRAW_NAMESPACE=${raw_namespace} -P ${PROJECT_SOURCE_DIR}/cmake/script/GenerateHeaderFromRaw.cmake
1825
DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/${raw_source_relpath} ${PROJECT_SOURCE_DIR}/cmake/script/GenerateHeaderFromRaw.cmake
1926
VERBATIM
27+
${DEPENDS_EXPLICIT_OPT}
2028
)
2129
endfunction()

src/util/CMakeLists.txt

+3
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,9 @@ add_library(bitcoin_util STATIC EXCLUDE_FROM_ALL
3636
../sync.cpp
3737
)
3838

39+
# Workaround for https://gitlab.kitware.com/cmake/cmake/-/issues/24058
40+
set_target_properties(bitcoin_util PROPERTIES OPTIMIZE_DEPENDENCIES OFF)
41+
3942
target_link_libraries(bitcoin_util
4043
PRIVATE
4144
core_interface

0 commit comments

Comments
 (0)