Skip to content

Conversation

@l0rinc
Copy link
Contributor

@l0rinc l0rinc commented Jan 24, 2025

When benchmarking IBD or reindex behavior we have to build bitcoind only, which currently looks like:

cmake -B build -DCMAKE_BUILD_TYPE=Release -DBUILD_CLI=OFF -DBUILD_TESTS=OFF -DBUILD_TX=OFF -DBUILD_UTIL=OFF -DENABLE_EXTERNAL_SIGNER=OFF -DENABLE_WALLET=OFF -DINSTALL_MAN=OFF

After this change we can simplify that to

cmake --preset bitcoind-release

You can validate the change by comparing the outputs of before and after:

git clean -fxd >/dev/null 2>&1 \
&& stdbuf -oL cmake -B build -DCMAKE_BUILD_TYPE=Release -DBUILD_CLI=OFF -DBUILD_TESTS=OFF -DBUILD_TX=OFF -DBUILD_UTIL=OFF -DENABLE_EXTERNAL_SIGNER=OFF -DENABLE_WALLET=OFF -DINSTALL_MAN=OFF 2>&1 | grep -E "( OFF| ON)|CMAKE_BUILD_TYPE"

vs

git clean -fxd >/dev/null 2>&1 \
&& stdbuf -oL cmake --preset bitcoind-release 2>&1 | grep -E "( OFF| ON)|CMAKE_BUILD_TYPE"

which will only contain a single CMAKE_BUILD_TYPE="Release" (but CMAKE_BUILD_TYPE ...................... Release should be present in both)

… with `release` build type

When benchmarking IBD or reindex behavior we have to build bitcoind only, which currently looks like:
  cmake -B build -DCMAKE_BUILD_TYPE=Release -DBUILD_CLI=OFF -DBUILD_TESTS=OFF -DBUILD_TX=OFF -DBUILD_UTIL=OFF -DENABLE_EXTERNAL_SIGNER=OFF -DENABLE_WALLET=OFF -DINSTALL_MAN=OFF
After this change we can simplify that to
  cmake --preset bitcoind-release
@DrahtBot
Copy link
Contributor

DrahtBot commented Jan 24, 2025

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/31730.

Reviews

See the guideline for information on the review process.
A summary of reviews will appear here.

@maflcko
Copy link
Member

maflcko commented Jan 24, 2025

Seems easier to just call $ cmake --build ./bld/ --target bitcoind? Otherwise there will be a preset for every binary?

@l0rinc
Copy link
Contributor Author

l0rinc commented Jan 24, 2025

I can confirm that with -DCMAKE_BUILD_TYPE=Release and -DENABLE_WALLET=OFF, --target bitcoind is indeed a replacement of the above, i.e.:

git clean -fxd \
&& cmake -B build -DCMAKE_BUILD_TYPE=Release -DENABLE_WALLET=OFF \
&& cmake --build build --target bitcoind

produces basically the same output during build phase as:

git clean -fxd \
&& cmake -B build -DCMAKE_BUILD_TYPE=Release -DBUILD_CLI=OFF -DBUILD_TESTS=OFF -DBUILD_TX=OFF -DBUILD_UTIL=OFF -DENABLE_EXTERNAL_SIGNER=OFF -DENABLE_WALLET=OFF -DINSTALL_MAN=OFF \
&& cmake --build build

Thanks!

@l0rinc l0rinc closed this Jan 24, 2025
@l0rinc l0rinc deleted the l0rinc/bitcoind-release-cmake-preset branch January 24, 2025 23:15
l0rinc added a commit to l0rinc/bitcoin that referenced this pull request Jan 25, 2025
l0rinc added a commit to bitcoin-dev-tools/benchcoin that referenced this pull request Jan 25, 2025
l0rinc added a commit to l0rinc/bitcoin that referenced this pull request Jan 28, 2025
l0rinc added a commit to bitcoin-dev-tools/benchcoin that referenced this pull request Jan 28, 2025
fanquake added a commit that referenced this pull request Feb 20, 2025
758a93d doc: update translation generation cmake example (Lőrinc)

Pull request description:

  While investigating #31730 I noticed that
  * the `dev-mode` preset already contained [`-DWITH_BDB=ON`](https://github.com/bitcoin/bitcoin/blob/master/CMakePresets.json#L83) and [`-DBUILD_GUI=ON`](https://github.com/bitcoin/bitcoin/blob/master/CMakePresets.json#L70);
  * the preset already contained a [default binary dir](https://github.com/bitcoin/bitcoin/blob/master/CMakePresets.json#L64) which we could use;
  * the command only runs on my Mac if we disable `USDT`, and `MULTIPROCESS` and on Linux also without `MULTIPROCESS`.

ACKs for top commit:
  hebasto:
    ACK 758a93d.

Tree-SHA512: f5bef99bff090f53dae04018f17e60655698fea23084f6a3d38affd830ca041d5c56466633206b63ae01e85c55b784d003e323b2de7c59028b595d4e8f50783c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants