mirror of
https://github.com/bitcoin/bitcoin.git
synced 2026-02-09 02:59:31 +08:00
Merge bitcoin/bitcoin#32497: merkle: pre‑reserve leaves to prevent reallocs with odd vtx count
3dd815f048validation: pre-reserve leaves to prevent reallocs with odd vtx count (Lőrinc)7fd47e0e56bench: make `MerkleRoot` benchmark more representative (Lőrinc)f0a2183108test: 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: ACK3dd815f048optout21: reACK3dd815f048hodlinator: re-ACK3dd815f048vasild: ACK3dd815f048w0xlt: ACK3dd815f048with minor nits. danielabrozzoni: Code review ACK3dd815f048Tree-SHA512: e7b578f9deadc0de7d61c062c7f65c5e1d347548ead4a4bb74b056396ad7df3f1c564327edc219670e6e2b2cb51f4e1ccfd4f58dd414aeadf2008d427065c11f
This commit is contained in:
@@ -7,21 +7,33 @@
|
||||
#include <random.h>
|
||||
#include <uint256.h>
|
||||
|
||||
#include <cassert>
|
||||
#include <vector>
|
||||
|
||||
static void MerkleRoot(benchmark::Bench& bench)
|
||||
{
|
||||
FastRandomContext rng(true);
|
||||
std::vector<uint256> leaves;
|
||||
leaves.resize(9001);
|
||||
for (auto& item : leaves) {
|
||||
FastRandomContext rng{/*fDeterministic=*/true};
|
||||
|
||||
std::vector<uint256> hashes{};
|
||||
hashes.resize(9001);
|
||||
for (auto& item : hashes) {
|
||||
item = rng.rand256();
|
||||
}
|
||||
bench.batch(leaves.size()).unit("leaf").run([&] {
|
||||
bool mutation = false;
|
||||
uint256 hash = ComputeMerkleRoot(std::vector<uint256>(leaves), &mutation);
|
||||
leaves[mutation] = hash;
|
||||
});
|
||||
|
||||
constexpr uint256 expected_root{"d8d4dfd014a533bc3941b8663fa6e7f3a8707af124f713164d75b0c3179ecb08"};
|
||||
for (bool mutate : {false, true}) {
|
||||
bench.name(mutate ? "MerkleRootWithMutation" : "MerkleRoot").batch(hashes.size()).unit("leaf").run([&] {
|
||||
std::vector<uint256> leaves;
|
||||
leaves.reserve((hashes.size() + 1) & ~1ULL); // capacity rounded up to even
|
||||
for (size_t s = 0; s < hashes.size(); s++) {
|
||||
leaves.push_back(hashes[s]);
|
||||
}
|
||||
|
||||
bool mutated{false};
|
||||
const uint256 root{ComputeMerkleRoot(std::move(leaves), mutate ? &mutated : nullptr)};
|
||||
assert(root == expected_root);
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
BENCHMARK(MerkleRoot);
|
||||
|
||||
@@ -66,9 +66,9 @@ uint256 ComputeMerkleRoot(std::vector<uint256> hashes, bool* mutated) {
|
||||
uint256 BlockMerkleRoot(const CBlock& block, bool* mutated)
|
||||
{
|
||||
std::vector<uint256> leaves;
|
||||
leaves.resize(block.vtx.size());
|
||||
leaves.reserve((block.vtx.size() + 1) & ~1ULL); // capacity rounded up to even
|
||||
for (size_t s = 0; s < block.vtx.size(); s++) {
|
||||
leaves[s] = block.vtx[s]->GetHash().ToUint256();
|
||||
leaves.push_back(block.vtx[s]->GetHash().ToUint256());
|
||||
}
|
||||
return ComputeMerkleRoot(std::move(leaves), mutated);
|
||||
}
|
||||
@@ -76,10 +76,10 @@ uint256 BlockMerkleRoot(const CBlock& block, bool* mutated)
|
||||
uint256 BlockWitnessMerkleRoot(const CBlock& block)
|
||||
{
|
||||
std::vector<uint256> leaves;
|
||||
leaves.resize(block.vtx.size());
|
||||
leaves[0].SetNull(); // The witness hash of the coinbase is 0.
|
||||
leaves.reserve((block.vtx.size() + 1) & ~1ULL); // capacity rounded up to even
|
||||
leaves.emplace_back(); // The witness hash of the coinbase is 0.
|
||||
for (size_t s = 1; s < block.vtx.size(); s++) {
|
||||
leaves[s] = block.vtx[s]->GetWitnessHash().ToUint256();
|
||||
leaves.push_back(block.vtx[s]->GetWitnessHash().ToUint256());
|
||||
}
|
||||
return ComputeMerkleRoot(std::move(leaves));
|
||||
}
|
||||
|
||||
@@ -59,10 +59,10 @@ static bool FetchAndClearCommitmentSection(const std::span<const uint8_t> header
|
||||
static uint256 ComputeModifiedMerkleRoot(const CMutableTransaction& cb, const CBlock& block)
|
||||
{
|
||||
std::vector<uint256> leaves;
|
||||
leaves.resize(block.vtx.size());
|
||||
leaves[0] = cb.GetHash().ToUint256();
|
||||
leaves.reserve((block.vtx.size() + 1) & ~1ULL); // capacity rounded up to even
|
||||
leaves.push_back(cb.GetHash().ToUint256());
|
||||
for (size_t s = 1; s < block.vtx.size(); ++s) {
|
||||
leaves[s] = block.vtx[s]->GetHash().ToUint256();
|
||||
leaves.push_back(block.vtx[s]->GetHash().ToUint256());
|
||||
}
|
||||
return ComputeMerkleRoot(std::move(leaves));
|
||||
}
|
||||
|
||||
@@ -80,8 +80,8 @@ FUZZ_TARGET(integer, .init = initialize_integer)
|
||||
}
|
||||
constexpr uint256 u256_min{"0000000000000000000000000000000000000000000000000000000000000000"};
|
||||
constexpr uint256 u256_max{"ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"};
|
||||
const std::vector<uint256> v256{u256, u256_min, u256_max};
|
||||
(void)ComputeMerkleRoot(v256);
|
||||
std::vector v256{u256, u256_min, u256_max};
|
||||
(void)ComputeMerkleRoot(std::move(v256));
|
||||
(void)DecompressAmount(u64);
|
||||
{
|
||||
if (std::optional<CAmount> parsed = ParseMoney(FormatMoney(i64))) {
|
||||
|
||||
@@ -232,8 +232,9 @@ BOOST_AUTO_TEST_CASE(merkle_test_BlockWitness)
|
||||
{
|
||||
CBlock block;
|
||||
|
||||
block.vtx.resize(2);
|
||||
for (std::size_t pos = 0; pos < block.vtx.size(); pos++) {
|
||||
constexpr size_t vtx_count{3};
|
||||
block.vtx.resize(vtx_count);
|
||||
for (std::size_t pos = 0; pos < vtx_count; pos++) {
|
||||
CMutableTransaction mtx;
|
||||
mtx.nLockTime = pos;
|
||||
block.vtx[pos] = MakeTransactionRef(std::move(mtx));
|
||||
@@ -242,12 +243,12 @@ BOOST_AUTO_TEST_CASE(merkle_test_BlockWitness)
|
||||
uint256 blockWitness = BlockWitnessMerkleRoot(block);
|
||||
|
||||
std::vector<uint256> hashes;
|
||||
hashes.resize(block.vtx.size());
|
||||
hashes[0].SetNull();
|
||||
hashes[1] = block.vtx[1]->GetHash().ToUint256();
|
||||
hashes.resize(vtx_count); // Note: leaving odd count to exercise old behavior
|
||||
for (size_t pos{1}; pos < vtx_count; ++pos) {
|
||||
hashes[pos] = block.vtx[pos]->GetWitnessHash().ToUint256();
|
||||
}
|
||||
|
||||
uint256 merkleRootofHashes = ComputeMerkleRoot(hashes);
|
||||
|
||||
BOOST_CHECK_EQUAL(merkleRootofHashes, blockWitness);
|
||||
}
|
||||
BOOST_AUTO_TEST_SUITE_END()
|
||||
|
||||
Reference in New Issue
Block a user