Commit Graph

27 Commits

Author SHA1 Message Date
Ava Chow
8c07800b19 Merge bitcoin/bitcoin#32497: merkle: pre‑reserve leaves to prevent reallocs with odd vtx count
3dd815f048 validation: pre-reserve leaves to prevent reallocs with odd vtx count (Lőrinc)
7fd47e0e56 bench: make `MerkleRoot` benchmark more representative (Lőrinc)
f0a2183108 test: adjust `ComputeMerkleRoot` tests (Lőrinc)

Pull request description:

  #### Summary

  `ComputeMerkleRoot` [duplicates the last hash](39b6c139bd/src/consensus/merkle.cpp (L54-L56)) when the input size is odd. If the caller provides a `std::vector` whose capacity equals its size, that extra `push_back` forces a reallocation, doubling its capacity (causing peak memory usage of 3x the necessary size).

  This affects roughly half of the created blocks (those with odd transaction counts), causing unnecessary memory fragmentation during every block validation.

  #### Fix

  * Pre-reserves vector capacity to account for the odd-count duplication using `(size + 1) & ~1ULL`.
      * This syntax produces [optimal assembly](https://github.com/bitcoin/bitcoin/pull/32497#discussion_r2553107836) across x86/ARM and 32/64-bit platforms for GCC & Clang.
  * Eliminates default construction of `uint256` objects that are immediately overwritten by switching from `resize` to `reserve` + `push_back`.

  #### Memory Impact

  [Memory profiling](https://github.com/bitcoin/bitcoin/pull/32497#issuecomment-3563724551) shows **50% reduction in peak allocation** (576KB → 288KB) and elimination of reallocation overhead.

  #### Validation

  The benchmark was updated to use an odd leaf count to demonstrate the real-world scenario where the reallocation occurs.

  A full `-reindex-chainstate` up to block **896 408** ran without triggering the asserts.

  <details>
  <summary>Validation asserts</summary>

  Temporary asserts (not included in this PR) confirm that `push_back` never reallocates and that the coinbase witness hash remains null:
  ```cpp
  if (hashes.size() & 1) {
      assert(hashes.size() < hashes.capacity()); // TODO remove
      hashes.push_back(hashes.back());
  }

  leaves.reserve((block.vtx.size() + 1) & ~1ULL); // capacity rounded up to even
  leaves.emplace_back();
  assert(leaves.back().IsNull()); // TODO remove
  ```

  </details>

  #### Benchmark Performance

  While the main purpose is to improve predictability, the reduced memory operations also improve hashing throughput slightly.

ACKs for top commit:
  achow101:
    ACK 3dd815f048
  optout21:
    reACK 3dd815f048
  hodlinator:
    re-ACK 3dd815f048
  vasild:
    ACK 3dd815f048
  w0xlt:
    ACK 3dd815f048 with minor nits.
  danielabrozzoni:
    Code review ACK 3dd815f048

Tree-SHA512: e7b578f9deadc0de7d61c062c7f65c5e1d347548ead4a4bb74b056396ad7df3f1c564327edc219670e6e2b2cb51f4e1ccfd4f58dd414aeadf2008d427065c11f
2026-01-20 15:47:17 -08:00
Lőrinc
3dd815f048 validation: pre-reserve leaves to prevent reallocs with odd vtx count
`ComputeMerkleRoot` duplicates the last hash when the input size is odd. If the caller provides a `std::vector` whose capacity equals its size, that extra `push_back` forces a reallocation, doubling its capacity (allocating 3x the necessary memory).

This affects roughly half of the created blocks (those with odd transaction counts), causing unnecessary memory fragmentation during every block validation.

Fix this by pre-reserving the vector capacity to account for the odd-count duplication. The expression `(size + 1) & ~1ULL` adds 1 to the size and clears the last bit, effectively rounding up to the next even number. This syntax produces optimal assembly across x86/ARM and 32/64-bit platforms for gcc/clang, see https://godbolt.org/z/xzscoq7nv.

Also switch from `resize` to `reserve` + `push_back` to eliminate the default construction of `uint256` objects that are immediately overwritten.

> ./build/bin/bench_bitcoin -filter='MerkleRoot.*' -min-time=1000

|             ns/leaf |              leaf/s |    err% |     total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
|               43.73 |       22,867,350.51 |    0.0% |      1.10 | `MerkleRoot`
|               44.17 |       22,640,349.14 |    0.0% |      1.10 | `MerkleRootWithMutation`

Massif memory measurements after show 0.8 MB peak memory usage

    KB
801.4^ #
     | #
     | #
     | #
     | #
     | #
     | #
     | #                                                         :::::@:::::@:
     | #:::@@@::@:::::::::::::::@::@:@:::@@:::::::::@::::::@:::::@::::@:::::@:
     | #:::@ @: @:::::::::::::::@::@:@:::@ :::: ::::@::::::@:::::@::::@:::::@:
     | #:::@ @: @:::::::::::::::@::@:@:::@ :::: ::::@::::::@:::::@::::@:::::@:
     | #:::@ @: @:::::::::::::::@::@:@:::@ :::: ::::@::::::@:::::@::::@:::::@:
     | #:::@ @: @:::::::::::::::@::@:@:::@ :::: ::::@::::::@:::::@::::@:::::@:
     | #:::@ @: @:::::::::::::::@::@:@:::@ :::: ::::@::::::@:::::@::::@:::::@:
     | #:::@ @: @:::::::::::::::@::@:@:::@ :::: ::::@::::::@:::::@::::@:::::@:
     | #:::@ @: @:::::::::::::::@::@:@:::@ :::: ::::@::::::@:::::@::::@:::::@:
     | #:::@ @: @:::::::::::::::@::@:@:::@ :::: ::::@::::::@:::::@::::@:::::@:
     | #:::@ @: @:::::::::::::::@::@:@:::@ :::: ::::@::::::@:::::@::::@:::::@:
     | #:::@ @: @:::::::::::::::@::@:@:::@ :::: ::::@::::::@:::::@::::@:::::@:
     | #:::@ @: @:::::::::::::::@::@:@:::@ :::: ::::@::::::@:::::@::::@:::::@:
   0 +----------------------------------------------------------------------->s
     0                                                                   227.5

and the stacks don't show reallocs anymore:
96.37% (790,809B) (heap allocation functions) malloc/new/new[], --alloc-fns, etc.
->35.10% (288,064B) 0x2234AF: allocate (new_allocator.h:151)
| ->35.10% (288,064B) 0x2234AF: allocate (allocator.h:203)
|   ->35.10% (288,064B) 0x2234AF: allocate (alloc_traits.h:614)
|     ->35.10% (288,064B) 0x2234AF: _M_allocate (stl_vector.h:387)
|       ->35.10% (288,064B) 0x2234AF: reserve (vector.tcc:79)
|         ->35.10% (288,064B) 0x2234AF: ToMerkleLeaves<std::vector<uint256>, MerkleRoot(ankerl::nanobench::Bench&)::<lambda()>::<lambda(bool, const auto:46&)> > (merkle.h:19)
|           ->35.10% (288,064B) 0x2234AF: operator() (merkle_root.cpp:25)
|             ->35.10% (288,064B) 0x2234AF: ankerl::nanobench::Bench& ankerl::nanobench::Bench::run<MerkleRoot(ankerl::nanobench::Bench&)::{lambda()

Co-authored-by: optout21 <13562139+optout21@users.noreply.github.com>
Co-authored-by: Hodlinator <172445034+hodlinator@users.noreply.github.com>
2025-12-23 09:44:59 +02:00
MarcoFalke
fa5f297748 scripted-diff: [doc] Unify stale copyright headers
-BEGIN VERIFY SCRIPT-

 sed --in-place --regexp-extended \
   's;( 20[0-2][0-9])(-20[0-2][0-9])? The Bitcoin Core developers;\1-present The Bitcoin Core developers;g' \
   $( git grep -l 'The Bitcoin Core developers' -- ':(exclude)COPYING' ':(exclude)src/ipc/libmultiprocess' ':(exclude)src/minisketch' )

-END VERIFY SCRIPT-
2025-12-16 22:21:15 +01:00
Lőrinc
24ed820d4f merkle: remove unused mutated arg from BlockWitnessMerkleRoot
The `mutated` parameter is never used at any call site - all callers pass `nullptr`.
The explicit comment in `validation.cpp` explains the reason:
// The malleation check is ignored; as the transaction tree itself
// already does not permit it, it is impossible to trigger in the
// witness tree.
2025-11-06 11:26:17 +01:00
Lőrinc
63d640fa6a merkle: remove unused proot and pmutated args from MerkleComputation
There's a single call to the methods from `ComputeMerklePath` where these were always set to `nullptr`.
2025-11-06 10:21:51 +01:00
Lőrinc
be270551df merkle: migrate path arg of MerkleComputation to a reference
There's a single call to the methods from `ComputeMerklePath` where the last parameter is always provided.
This simplifies the implementation by not having to check for missing parameter.
2025-11-06 10:20:50 +01:00
marcofleon
9c24cda72e refactor: Convert remaining instances from uint256 to Txid
These remaining miscellaneous changes were identified by commenting out
the `operator const uint256&` conversion and the `Compare(const uint256&)`
method from `transaction_identifier.h`.
2025-08-11 16:47:43 +01:00
Sjors Provoost
f86678156a Check leaves size maximum in MerkleComputation
Belt and suspenders for future code changes.

Currently this function is only called from TransactionMerklePath() which sets leaves to the block transactions, so the Assume always holds.
2024-12-17 10:12:31 +07:00
Sjors Provoost
39d3b538e6 Rename merkle branch to path 2024-12-17 10:12:31 +07:00
Sjors Provoost
63d6ad7c89 Move BlockMerkleBranch back to merkle.{h,cpp}
The Mining interface uses this function in the next commit
to calculate the coinbase merkle path. Stratum v2 uses
this to send a compact work template.

This partially undoes the change in 4defdfab94,
but is not a revert, because the implementation changed in the meantime.

This commit also documents the function.
2024-09-26 09:48:31 +02:00
MarcoFalke
fa488f131f scripted-diff: Bump copyright headers
-BEGIN VERIFY SCRIPT-
./contrib/devtools/copyright_header.py update ./
-END VERIFY SCRIPT-
2020-04-16 13:33:09 -04:00
4d55397500
5b59a19731 Update merkle.cpp
Change comment from `The reason is that if the number of hashes in the list at a given time
is odd`, to ` The reason is that if the number of hashes in the list at a given level
       is odd` (to be a bit more precise)
2020-03-18 10:34:53 -07:00
MarcoFalke
aaaaad6ac9 scripted-diff: Bump copyright of files changed in 2019
-BEGIN VERIFY SCRIPT-
./contrib/devtools/copyright_header.py update ./
-END VERIFY SCRIPT-
2019-12-30 10:42:20 +13:00
practicalswift
eca9767673 Make reasoning about dependencies easier by not including unused dependencies 2019-06-02 17:15:23 +02:00
Jim Posen
2068f089c8 scripted-diff: Move util files to separate directory.
-BEGIN VERIFY SCRIPT-
mkdir -p src/util
git mv src/util.h src/util/system.h
git mv src/util.cpp src/util/system.cpp
git mv src/utilmemory.h src/util/memory.h
git mv src/utilmoneystr.h src/util/moneystr.h
git mv src/utilmoneystr.cpp src/util/moneystr.cpp
git mv src/utilstrencodings.h src/util/strencodings.h
git mv src/utilstrencodings.cpp src/util/strencodings.cpp
git mv src/utiltime.h src/util/time.h
git mv src/utiltime.cpp src/util/time.cpp

sed -i 's/<util\.h>/<util\/system\.h>/g' $(git ls-files 'src/*.h' 'src/*.cpp')
sed -i 's/<utilmemory\.h>/<util\/memory\.h>/g' $(git ls-files 'src/*.h' 'src/*.cpp')
sed -i 's/<utilmoneystr\.h>/<util\/moneystr\.h>/g' $(git ls-files 'src/*.h' 'src/*.cpp')
sed -i 's/<utilstrencodings\.h>/<util\/strencodings\.h>/g' $(git ls-files 'src/*.h' 'src/*.cpp')
sed -i 's/<utiltime\.h>/<util\/time\.h>/g' $(git ls-files 'src/*.h' 'src/*.cpp')

sed -i 's/BITCOIN_UTIL_H/BITCOIN_UTIL_SYSTEM_H/g' src/util/system.h
sed -i 's/BITCOIN_UTILMEMORY_H/BITCOIN_UTIL_MEMORY_H/g' src/util/memory.h
sed -i 's/BITCOIN_UTILMONEYSTR_H/BITCOIN_UTIL_MONEYSTR_H/g' src/util/moneystr.h
sed -i 's/BITCOIN_UTILSTRENCODINGS_H/BITCOIN_UTIL_STRENCODINGS_H/g' src/util/strencodings.h
sed -i 's/BITCOIN_UTILTIME_H/BITCOIN_UTIL_TIME_H/g' src/util/time.h

sed -i 's/ util\.\(h\|cpp\)/ util\/system\.\1/g' src/Makefile.am
sed -i 's/utilmemory\.\(h\|cpp\)/util\/memory\.\1/g' src/Makefile.am
sed -i 's/utilmoneystr\.\(h\|cpp\)/util\/moneystr\.\1/g' src/Makefile.am
sed -i 's/utilstrencodings\.\(h\|cpp\)/util\/strencodings\.\1/g' src/Makefile.am
sed -i 's/utiltime\.\(h\|cpp\)/util\/time\.\1/g' src/Makefile.am

sed -i 's/-> util ->/-> util\/system ->/' test/lint/lint-circular-dependencies.sh
sed -i 's/src\/util\.cpp/src\/util\/system\.cpp/g' test/lint/lint-format-strings.py test/lint/lint-locale-dependence.sh
sed -i 's/src\/utilmoneystr\.cpp/src\/util\/moneystr\.cpp/g' test/lint/lint-locale-dependence.sh
sed -i 's/src\/utilstrencodings\.\(h\|cpp\)/src\/util\/strencodings\.\1/g' test/lint/lint-locale-dependence.sh
sed -i 's/src\\utilstrencodings\.cpp/src\\util\\strencodings\.cpp/' build_msvc/libbitcoinconsensus/libbitcoinconsensus.vcxproj
-END VERIFY SCRIPT-
2018-11-04 22:46:07 -08:00
DrahtBot
eb7daf4d60 Update copyright headers to 2018 2018-07-27 07:15:02 -04:00
Pieter Wuille
4defdfab94 [MOVEONLY] Move unused Merkle branch code to tests 2018-05-29 14:20:12 -07:00
Pieter Wuille
1f0e7ca09c Use SHA256D64 in Merkle root computation 2018-05-29 14:17:07 -07:00
Akira Takizawa
595a7bab23 Increment MIT Licence copyright header year on files modified in 2017 2018-01-03 02:26:56 +09:00
MeshCollider
1a445343f6 scripted-diff: Replace #include "" with #include <> (ryanofsky)
-BEGIN VERIFY SCRIPT-
for f in \
  src/*.cpp \
  src/*.h \
  src/bench/*.cpp \
  src/bench/*.h \
  src/compat/*.cpp \
  src/compat/*.h \
  src/consensus/*.cpp \
  src/consensus/*.h \
  src/crypto/*.cpp \
  src/crypto/*.h \
  src/crypto/ctaes/*.h \
  src/policy/*.cpp \
  src/policy/*.h \
  src/primitives/*.cpp \
  src/primitives/*.h \
  src/qt/*.cpp \
  src/qt/*.h \
  src/qt/test/*.cpp \
  src/qt/test/*.h \
  src/rpc/*.cpp \
  src/rpc/*.h \
  src/script/*.cpp \
  src/script/*.h \
  src/support/*.cpp \
  src/support/*.h \
  src/support/allocators/*.h \
  src/test/*.cpp \
  src/test/*.h \
  src/wallet/*.cpp \
  src/wallet/*.h \
  src/wallet/test/*.cpp \
  src/wallet/test/*.h \
  src/zmq/*.cpp \
  src/zmq/*.h
do
  base=${f%/*}/ relbase=${base#src/} sed -i "s:#include \"\(.*\)\"\(.*\):if test -e \$base'\\1'; then echo \"#include <\"\$relbase\"\\1>\\2\"; else echo \"#include <\\1>\\2\"; fi:e" $f
done
-END VERIFY SCRIPT-
2017-11-16 08:23:01 +13:00
practicalswift
90d4d89230 scripted-diff: Use the C++11 keyword nullptr to denote the pointer literal instead of the macro NULL
-BEGIN VERIFY SCRIPT-
sed -i 's/\<NULL\>/nullptr/g' src/*.cpp src/*.h src/*/*.cpp src/*/*.h src/qt/*/*.cpp src/qt/*/*.h src/wallet/*/*.cpp src/wallet/*/*.h src/support/allocators/*.h
sed -i 's/Prefer nullptr, otherwise SAFECOOKIE./Prefer NULL, otherwise SAFECOOKIE./g' src/torcontrol.cpp
sed -i 's/tor: Using nullptr authentication/tor: Using NULL authentication/g' src/torcontrol.cpp
sed -i 's/METHODS=nullptr/METHODS=NULL/g' src/test/torcontrol_tests.cpp src/torcontrol.cpp
sed -i 's/nullptr certificates/NULL certificates/g' src/qt/paymentserver.cpp
sed -i 's/"nullptr"/"NULL"/g' src/torcontrol.cpp src/test/torcontrol_tests.cpp
-END VERIFY SCRIPT-
2017-08-07 07:36:37 +02:00
isle2983
27765b6403 Increment MIT Licence copyright header year on files modified in 2016
Edited via:

$ contrib/devtools/copyright_header.py update .
2016-12-31 11:01:21 -07:00
Pieter Wuille
1662b437b3 Make CBlock::vtx a vector of shared_ptr<CTransaction> 2016-11-19 17:51:09 -08:00
Pieter Wuille
8b49040854 BIP141: Commitment structure and deployment
Includes a fix by Suhas Daftuar and LongShao007
2016-06-22 15:42:59 +02:00
MarcoFalke
fa60d05a4e Add missing copyright headers 2016-01-05 21:34:15 +01:00
Pieter Wuille
eece63fa72 Switch blocks to a constant-space Merkle root/branch algorithm.
This switches the Merkle tree logic for blocks to one that runs in constant (small) space.
The old code is moved to tests, and a new test is added that for various combinations of
block sizes, transaction positions to compute a branch for, and mutations:
 * Verifies that the old code and new code agree for the Merkle root.
 * Verifies that the old code and new code agree for the Merkle branch.
 * Verifies that the computed Merkle branch is valid.
 * Verifies that mutations don't change the Merkle root.
 * Verifies that mutations are correctly detected.
2015-11-27 15:36:52 +01:00
Pieter Wuille
ee60e5625b Add merkle.{h,cpp}, generic merkle root/branch algorithm 2015-11-27 15:31:01 +01:00