From 5db7d4d3d28bd1269a09955b4695135c86c4827d Mon Sep 17 00:00:00 2001 From: TheCharlatan Date: Wed, 11 Dec 2024 22:15:22 +0100 Subject: [PATCH 1/8] doc: Correct docstring describing max block tree db cache It is always applied in the same way, no matter how the txindex is setup. This was no longer accurate after 8181db8, where their initialization was made independent. --- src/txdb.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/txdb.h b/src/txdb.h index 565b060dbea..671cbd12866 100644 --- a/src/txdb.h +++ b/src/txdb.h @@ -27,11 +27,11 @@ static const int64_t nDefaultDbCache = 450; static const int64_t nDefaultDbBatchSize = 16 << 20; //! min. -dbcache (MiB) static const int64_t nMinDbCache = 4; -//! Max memory allocated to block tree DB specific cache, if no -txindex (MiB) +//! Max memory allocated to block tree DB specific cache (MiB) static const int64_t nMaxBlockDBCache = 2; -//! Max memory allocated to block tree DB specific cache, if -txindex (MiB) // Unlike for the UTXO database, for the txindex scenario the leveldb cache make // a meaningful difference: https://github.com/bitcoin/bitcoin/pull/8273#issuecomment-229601991 +//! Max memory allocated to tx index DB specific cache in MiB. static const int64_t nMaxTxIndexCache = 1024; //! Max memory allocated to all block filter index caches combined in MiB. static const int64_t max_filter_index_cache = 1024; From 8bd5f8a38ce903c05606841ebed1902398cb0b14 Mon Sep 17 00:00:00 2001 From: TheCharlatan Date: Tue, 17 Dec 2024 21:33:56 +0100 Subject: [PATCH 2/8] [refactor] init: Simplify coinsdb cache calculation (total_cache / 4) + (1 << 23) is at least 8 MiB and nMaxCoinsDBCache is also 8 MiB, so the minimum between the two will always be nMaxCoinsDBCache. This is just a simplification and not changing the result of the calculation. Co-authored-by: Ryan Ofsky --- src/node/caches.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/node/caches.cpp b/src/node/caches.cpp index dc4d98f592a..01b08e320d7 100644 --- a/src/node/caches.cpp +++ b/src/node/caches.cpp @@ -24,8 +24,7 @@ CacheSizes CalculateCacheSizes(const ArgsManager& args, size_t n_indexes) sizes.filter_index = max_cache / n_indexes; nTotalCache -= sizes.filter_index * n_indexes; } - sizes.coins_db = std::min(nTotalCache / 2, (nTotalCache / 4) + (1 << 23)); // use 25%-50% of the remainder for disk cache - sizes.coins_db = std::min(sizes.coins_db, nMaxCoinsDBCache << 20); // cap total coins db cache + sizes.coins_db = std::min(nTotalCache / 2, nMaxCoinsDBCache << 20); nTotalCache -= sizes.coins_db; sizes.coins = nTotalCache; // the rest goes to in-memory cache return sizes; From c03a2795a8e044d17835bbf03de0c64dc7b41da8 Mon Sep 17 00:00:00 2001 From: TheCharlatan Date: Thu, 9 Jan 2025 11:25:43 +0100 Subject: [PATCH 3/8] util: Add integer left shift helpers The helpers are used in the following commits to increase the safety of conversions during cache size calculations. Co-authored-by: Ryan Ofsky Co-authored-by: stickies-v --- src/test/util_tests.cpp | 97 +++++++++++++++++++++++++++++++++++++++++ src/util/byte_units.h | 22 ++++++++++ src/util/overflow.h | 36 +++++++++++++++ 3 files changed, 155 insertions(+) create mode 100644 src/util/byte_units.h diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp index 3f6e5b1b661..129e84bc240 100644 --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -13,6 +13,7 @@ #include #include #include +#include #include #include #include @@ -1877,4 +1878,100 @@ BOOST_AUTO_TEST_CASE(clearshrink_test) } } +template +void TestCheckedLeftShift() +{ + constexpr auto MAX{std::numeric_limits::max()}; + + // Basic operations + BOOST_CHECK_EQUAL(CheckedLeftShift(0, 1), 0); + BOOST_CHECK_EQUAL(CheckedLeftShift(0, 127), 0); + BOOST_CHECK_EQUAL(CheckedLeftShift(1, 1), 2); + BOOST_CHECK_EQUAL(CheckedLeftShift(2, 2), 8); + BOOST_CHECK_EQUAL(CheckedLeftShift(MAX >> 1, 1), MAX - 1); + + // Max left shift + BOOST_CHECK_EQUAL(CheckedLeftShift(1, std::numeric_limits::digits - 1), MAX / 2 + 1); + + // Overflow cases + BOOST_CHECK(!CheckedLeftShift((MAX >> 1) + 1, 1)); + BOOST_CHECK(!CheckedLeftShift(MAX, 1)); + BOOST_CHECK(!CheckedLeftShift(1, std::numeric_limits::digits)); + BOOST_CHECK(!CheckedLeftShift(1, std::numeric_limits::digits + 1)); + + if constexpr (std::is_signed_v) { + constexpr auto MIN{std::numeric_limits::min()}; + // Negative input + BOOST_CHECK_EQUAL(CheckedLeftShift(-1, 1), -2); + BOOST_CHECK_EQUAL(CheckedLeftShift((MIN >> 2), 1), MIN / 2); + BOOST_CHECK_EQUAL(CheckedLeftShift((MIN >> 1) + 1, 1), MIN + 2); + BOOST_CHECK_EQUAL(CheckedLeftShift(MIN >> 1, 1), MIN); + // Overflow negative + BOOST_CHECK(!CheckedLeftShift((MIN >> 1) - 1, 1)); + BOOST_CHECK(!CheckedLeftShift(MIN >> 1, 2)); + BOOST_CHECK(!CheckedLeftShift(-1, 100)); + } +} + +template +void TestSaturatingLeftShift() +{ + constexpr auto MAX{std::numeric_limits::max()}; + + // Basic operations + BOOST_CHECK_EQUAL(SaturatingLeftShift(0, 1), 0); + BOOST_CHECK_EQUAL(SaturatingLeftShift(0, 127), 0); + BOOST_CHECK_EQUAL(SaturatingLeftShift(1, 1), 2); + BOOST_CHECK_EQUAL(SaturatingLeftShift(2, 2), 8); + BOOST_CHECK_EQUAL(SaturatingLeftShift(MAX >> 1, 1), MAX - 1); + + // Max left shift + BOOST_CHECK_EQUAL(SaturatingLeftShift(1, std::numeric_limits::digits - 1), MAX / 2 + 1); + + // Saturation cases + BOOST_CHECK_EQUAL(SaturatingLeftShift((MAX >> 1) + 1, 1), MAX); + BOOST_CHECK_EQUAL(SaturatingLeftShift(MAX, 1), MAX); + BOOST_CHECK_EQUAL(SaturatingLeftShift(1, std::numeric_limits::digits), MAX); + BOOST_CHECK_EQUAL(SaturatingLeftShift(1, std::numeric_limits::digits + 1), MAX); + + if constexpr (std::is_signed_v) { + constexpr auto MIN{std::numeric_limits::min()}; + // Negative input + BOOST_CHECK_EQUAL(SaturatingLeftShift(-1, 1), -2); + BOOST_CHECK_EQUAL(SaturatingLeftShift((MIN >> 2), 1), MIN / 2); + BOOST_CHECK_EQUAL(SaturatingLeftShift((MIN >> 1) + 1, 1), MIN + 2); + BOOST_CHECK_EQUAL(SaturatingLeftShift(MIN >> 1, 1), MIN); + // Saturation negative + BOOST_CHECK_EQUAL(SaturatingLeftShift((MIN >> 1) - 1, 1), MIN); + BOOST_CHECK_EQUAL(SaturatingLeftShift(MIN >> 1, 2), MIN); + BOOST_CHECK_EQUAL(SaturatingLeftShift(-1, 100), MIN); + } +} + +BOOST_AUTO_TEST_CASE(checked_left_shift_test) +{ + TestCheckedLeftShift(); + TestCheckedLeftShift(); + TestCheckedLeftShift(); + TestCheckedLeftShift(); + TestCheckedLeftShift(); +} + +BOOST_AUTO_TEST_CASE(saturating_left_shift_test) +{ + TestSaturatingLeftShift(); + TestSaturatingLeftShift(); + TestSaturatingLeftShift(); + TestSaturatingLeftShift(); + TestSaturatingLeftShift(); +} + +BOOST_AUTO_TEST_CASE(mib_string_literal_test) +{ + BOOST_CHECK_EQUAL(0_MiB, 0); + BOOST_CHECK_EQUAL(1_MiB, 1024 * 1024); + const auto max_mib{std::numeric_limits::max() >> 20}; + BOOST_CHECK_EXCEPTION(operator""_MiB(static_cast(max_mib) + 1), std::overflow_error, HasReason("MiB value too large for size_t byte conversion")); +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/src/util/byte_units.h b/src/util/byte_units.h new file mode 100644 index 00000000000..894f057a7e4 --- /dev/null +++ b/src/util/byte_units.h @@ -0,0 +1,22 @@ +// Copyright (c) 2025-present The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_UTIL_BYTE_UNITS_H +#define BITCOIN_UTIL_BYTE_UNITS_H + +#include + +#include + +//! Overflow-safe conversion of MiB to bytes. +constexpr size_t operator"" _MiB(unsigned long long mebibytes) +{ + auto bytes{CheckedLeftShift(mebibytes, 20)}; + if (!bytes || *bytes > std::numeric_limits::max()) { + throw std::overflow_error("MiB value too large for size_t byte conversion"); + } + return *bytes; +} + +#endif // BITCOIN_UTIL_BYTE_UNITS_H diff --git a/src/util/overflow.h b/src/util/overflow.h index 7e0cce6c272..67711af0a5e 100644 --- a/src/util/overflow.h +++ b/src/util/overflow.h @@ -5,6 +5,8 @@ #ifndef BITCOIN_UTIL_OVERFLOW_H #define BITCOIN_UTIL_OVERFLOW_H +#include +#include #include #include #include @@ -47,4 +49,38 @@ template return i + j; } +/** + * @brief Left bit shift with overflow checking. + * @param input The input value to be left shifted. + * @param shift The number of bits to left shift. + * @return (input * 2^shift) or nullopt if it would not fit in the return type. + */ +template +constexpr std::optional CheckedLeftShift(T input, unsigned shift) noexcept +{ + if (shift == 0 || input == 0) return input; + // Avoid undefined c++ behaviour if shift is >= number of bits in T. + if (shift >= sizeof(T) * CHAR_BIT) return std::nullopt; + // If input << shift is too big to fit in T, return nullopt. + if (input > (std::numeric_limits::max() >> shift)) return std::nullopt; + if (input < (std::numeric_limits::min() >> shift)) return std::nullopt; + return input << shift; +} + +/** + * @brief Left bit shift with safe minimum and maximum values. + * @param input The input value to be left shifted. + * @param shift The number of bits to left shift. + * @return (input * 2^shift) clamped to fit between the lowest and highest + * representable values of the type T. + */ +template +constexpr T SaturatingLeftShift(T input, unsigned shift) noexcept +{ + if (auto result{CheckedLeftShift(input, shift)}) return *result; + // If input << shift is too big to fit in T, return biggest positive or negative + // number that fits. + return input < 0 ? std::numeric_limits::min() : std::numeric_limits::max(); +} + #endif // BITCOIN_UTIL_OVERFLOW_H From d5e2c4a4097c799433cfc5367c61568fad2c784e Mon Sep 17 00:00:00 2001 From: TheCharlatan Date: Tue, 14 Jan 2025 22:21:45 +0100 Subject: [PATCH 4/8] fuzz: Add fuzz test for checked and saturating add and left shift Co-authored-by: Ryan Ofsky --- src/test/fuzz/CMakeLists.txt | 1 + src/test/fuzz/overflow.cpp | 50 ++++++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+) create mode 100644 src/test/fuzz/overflow.cpp diff --git a/src/test/fuzz/CMakeLists.txt b/src/test/fuzz/CMakeLists.txt index f65ed62b2d0..a261d3ecea2 100644 --- a/src/test/fuzz/CMakeLists.txt +++ b/src/test/fuzz/CMakeLists.txt @@ -71,6 +71,7 @@ add_executable(fuzz netaddress.cpp netbase_dns_lookup.cpp node_eviction.cpp + overflow.cpp p2p_handshake.cpp p2p_headers_presync.cpp p2p_transport_serialization.cpp diff --git a/src/test/fuzz/overflow.cpp b/src/test/fuzz/overflow.cpp new file mode 100644 index 00000000000..0278bd0f415 --- /dev/null +++ b/src/test/fuzz/overflow.cpp @@ -0,0 +1,50 @@ +// Copyright (c) 2025-present The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include +#include +#include +#include + +#include +#include +#include + +namespace { +//! Test overflow operations for type T using a wider type, W, to verify results. +template +void TestOverflow(FuzzedDataProvider& fuzzed_data_provider) +{ + constexpr auto min{std::numeric_limits::min()}; + constexpr auto max{std::numeric_limits::max()}; + // Range needs to be at least twice as big to allow two numbers to be added without overflowing. + static_assert(min >= std::numeric_limits::min() / 2); + static_assert(max <= std::numeric_limits::max() / 2); + + auto widen = [](T value) -> W { return value; }; + auto clamp = [](W value) -> W { return std::clamp(value, min, max); }; + auto check = [](W value) -> std::optional { if (value >= min && value <= max) return value; else return std::nullopt; }; + + const T i = fuzzed_data_provider.ConsumeIntegral(); + const T j = fuzzed_data_provider.ConsumeIntegral(); + const unsigned shift = fuzzed_data_provider.ConsumeIntegralInRange(0, std::numeric_limits::digits - std::numeric_limits::digits); + + Assert(clamp(widen(i) + widen(j)) == SaturatingAdd(i, j)); + Assert(check(widen(i) + widen(j)) == CheckedAdd(i, j)); + + Assert(clamp(widen(i) << shift) == SaturatingLeftShift(i, shift)); + Assert(check(widen(i) << shift) == CheckedLeftShift(i, shift)); +} +} // namespace + +FUZZ_TARGET(overflow) +{ + FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); + TestOverflow(fuzzed_data_provider); + TestOverflow(fuzzed_data_provider); + TestOverflow(fuzzed_data_provider); + TestOverflow(fuzzed_data_provider); + TestOverflow(fuzzed_data_provider); + TestOverflow(fuzzed_data_provider); +} From e758b26b85da27ef44f3d2c924f3f08e8c1f4fdf Mon Sep 17 00:00:00 2001 From: TheCharlatan Date: Fri, 22 Nov 2024 20:51:34 +0100 Subject: [PATCH 5/8] kernel: Move kernel-specific cache size options to kernel Carrying non-kernel related fields in the cache sizes for the indexes is confusing for kernel library users. The cache sizes also are set currently with magic numbers in bitcoin-chainstate. The comments for the cache size calculations are also not completely clear. Solve these things by moving the kernel-specific cache size fields to their own struct. This slightly changes the way the cache is allocated if the txindex and/or blockfilterindex is used. Since they are now given precedence over the block tree db cache, this results in a bit less cache being allocated to the block tree db, coinsdb and coins caches. The effect is negligible though, i.e. cache sizes with default dbcache reported through the logs are: master: Cache configuration: * Using 2.0 MiB for block index database * Using 56.0 MiB for transaction index database * Using 49.0 MiB for basic block filter index database * Using 8.0 MiB for chain state database * Using 335.0 MiB for in-memory UTXO set (plus up to 286.1 MiB of unused mempool space) this branch: Cache configuration: * Using 2.0 MiB for block index database * Using 56.2 MiB for transaction index database * Using 49.2 MiB for basic block filter index database * Using 8.0 MiB for chain state database * Using 334.5 MiB for in-memory UTXO set (plus up to 286.1 MiB of unused mempool space) --- src/bitcoin-chainstate.cpp | 5 +---- src/init.cpp | 26 +++++++++++++------------- src/kernel/caches.h | 34 ++++++++++++++++++++++++++++++++++ src/node/caches.cpp | 14 ++++++-------- src/node/caches.h | 13 ++++++++----- src/node/chainstate.cpp | 4 +++- src/node/chainstate.h | 8 +++++--- src/test/util/setup_common.cpp | 7 ++----- src/test/util/setup_common.h | 3 ++- src/txdb.h | 4 ---- 10 files changed, 74 insertions(+), 44 deletions(-) create mode 100644 src/kernel/caches.h diff --git a/src/bitcoin-chainstate.cpp b/src/bitcoin-chainstate.cpp index bab09f13d9b..3b551e5c178 100644 --- a/src/bitcoin-chainstate.cpp +++ b/src/bitcoin-chainstate.cpp @@ -123,10 +123,7 @@ int main(int argc, char* argv[]) util::SignalInterrupt interrupt; ChainstateManager chainman{interrupt, chainman_opts, blockman_opts}; - node::CacheSizes cache_sizes; - cache_sizes.block_tree_db = 2 << 20; - cache_sizes.coins_db = 2 << 22; - cache_sizes.coins = (450 << 20) - (2 << 20) - (2 << 22); + kernel::CacheSizes cache_sizes{nDefaultDbCache << 20}; node::ChainstateLoadOptions options; auto [status, error] = node::LoadChainstate(chainman, cache_sizes, options); if (status != node::ChainstateLoadStatus::SUCCESS) { diff --git a/src/init.cpp b/src/init.cpp index a4ad2771cc7..878e11518b0 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -32,6 +32,7 @@ #include #include #include +#include #include #include #include @@ -121,7 +122,6 @@ using common::ResolveErrMsg; using node::ApplyArgsManOptions; using node::BlockManager; -using node::CacheSizes; using node::CalculateCacheSizes; using node::ChainstateLoadResult; using node::ChainstateLoadStatus; @@ -1177,7 +1177,7 @@ static ChainstateLoadResult InitAndLoadChainstate( NodeContext& node, bool do_reindex, const bool do_reindex_chainstate, - CacheSizes& cache_sizes, + const kernel::CacheSizes& cache_sizes, const ArgsManager& args) { const CChainParams& chainparams = Params(); @@ -1603,18 +1603,18 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) ReadNotificationArgs(args, kernel_notifications); // cache size calculations - CacheSizes cache_sizes = CalculateCacheSizes(args, g_enabled_filter_types.size()); + const auto [index_cache_sizes, kernel_cache_sizes] = CalculateCacheSizes(args, g_enabled_filter_types.size()); - LogPrintf("Cache configuration:\n"); - LogPrintf("* Using %.1f MiB for block index database\n", cache_sizes.block_tree_db * (1.0 / 1024 / 1024)); + LogInfo("Cache configuration:"); + LogInfo("* Using %.1f MiB for block index database", kernel_cache_sizes.block_tree_db * (1.0 / 1024 / 1024)); if (args.GetBoolArg("-txindex", DEFAULT_TXINDEX)) { - LogPrintf("* Using %.1f MiB for transaction index database\n", cache_sizes.tx_index * (1.0 / 1024 / 1024)); + LogInfo("* Using %.1f MiB for transaction index database", index_cache_sizes.tx_index * (1.0 / 1024 / 1024)); } for (BlockFilterType filter_type : g_enabled_filter_types) { - LogPrintf("* Using %.1f MiB for %s block filter index database\n", - cache_sizes.filter_index * (1.0 / 1024 / 1024), BlockFilterTypeName(filter_type)); + LogInfo("* Using %.1f MiB for %s block filter index database", + index_cache_sizes.filter_index * (1.0 / 1024 / 1024), BlockFilterTypeName(filter_type)); } - LogPrintf("* Using %.1f MiB for chain state database\n", cache_sizes.coins_db * (1.0 / 1024 / 1024)); + LogInfo("* Using %.1f MiB for chain state database", kernel_cache_sizes.coins_db * (1.0 / 1024 / 1024)); assert(!node.mempool); assert(!node.chainman); @@ -1627,7 +1627,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) node, do_reindex, do_reindex_chainstate, - cache_sizes, + kernel_cache_sizes, args); if (status == ChainstateLoadStatus::FAILURE && !do_reindex && !ShutdownRequested(node)) { // suggest a reindex @@ -1646,7 +1646,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) node, do_reindex, do_reindex_chainstate, - cache_sizes, + kernel_cache_sizes, args); } if (status != ChainstateLoadStatus::SUCCESS && status != ChainstateLoadStatus::INTERRUPTED) { @@ -1672,12 +1672,12 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) // ********************************************************* Step 8: start indexers if (args.GetBoolArg("-txindex", DEFAULT_TXINDEX)) { - g_txindex = std::make_unique(interfaces::MakeChain(node), cache_sizes.tx_index, false, do_reindex); + g_txindex = std::make_unique(interfaces::MakeChain(node), index_cache_sizes.tx_index, false, do_reindex); node.indexes.emplace_back(g_txindex.get()); } for (const auto& filter_type : g_enabled_filter_types) { - InitBlockFilterIndex([&]{ return interfaces::MakeChain(node); }, filter_type, cache_sizes.filter_index, false, do_reindex); + InitBlockFilterIndex([&]{ return interfaces::MakeChain(node); }, filter_type, index_cache_sizes.filter_index, false, do_reindex); node.indexes.emplace_back(GetBlockFilterIndex(filter_type)); } diff --git a/src/kernel/caches.h b/src/kernel/caches.h new file mode 100644 index 00000000000..2eab84df6be --- /dev/null +++ b/src/kernel/caches.h @@ -0,0 +1,34 @@ +// Copyright (c) 2024-present The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_KERNEL_CACHES_H +#define BITCOIN_KERNEL_CACHES_H + +#include + +#include + +//! Max memory allocated to block tree DB specific cache (bytes) +static constexpr size_t MAX_BLOCK_DB_CACHE{2_MiB}; +//! Max memory allocated to coin DB specific cache (bytes) +static constexpr size_t MAX_COINS_DB_CACHE{8_MiB}; + +namespace kernel { +struct CacheSizes { + size_t block_tree_db; + size_t coins_db; + size_t coins; + + CacheSizes(size_t total_cache) + { + block_tree_db = std::min(total_cache / 8, MAX_BLOCK_DB_CACHE); + total_cache -= block_tree_db; + coins_db = std::min(total_cache / 2, MAX_COINS_DB_CACHE); + total_cache -= coins_db; + coins = total_cache; // the rest goes to the coins cache + } +}; +} // namespace kernel + +#endif // BITCOIN_KERNEL_CACHES_H diff --git a/src/node/caches.cpp b/src/node/caches.cpp index 01b08e320d7..50b8bceebe8 100644 --- a/src/node/caches.cpp +++ b/src/node/caches.cpp @@ -6,27 +6,25 @@ #include #include +#include #include +#include +#include + namespace node { CacheSizes CalculateCacheSizes(const ArgsManager& args, size_t n_indexes) { int64_t nTotalCache = (args.GetIntArg("-dbcache", nDefaultDbCache) << 20); nTotalCache = std::max(nTotalCache, nMinDbCache << 20); // total cache cannot be less than nMinDbCache - CacheSizes sizes; - sizes.block_tree_db = std::min(nTotalCache / 8, nMaxBlockDBCache << 20); - nTotalCache -= sizes.block_tree_db; + IndexCacheSizes sizes; sizes.tx_index = std::min(nTotalCache / 8, args.GetBoolArg("-txindex", DEFAULT_TXINDEX) ? nMaxTxIndexCache << 20 : 0); nTotalCache -= sizes.tx_index; - sizes.filter_index = 0; if (n_indexes > 0) { int64_t max_cache = std::min(nTotalCache / 8, max_filter_index_cache << 20); sizes.filter_index = max_cache / n_indexes; nTotalCache -= sizes.filter_index * n_indexes; } - sizes.coins_db = std::min(nTotalCache / 2, nMaxCoinsDBCache << 20); - nTotalCache -= sizes.coins_db; - sizes.coins = nTotalCache; // the rest goes to in-memory cache - return sizes; + return {sizes, kernel::CacheSizes{static_cast(nTotalCache)}}; } } // namespace node diff --git a/src/node/caches.h b/src/node/caches.h index 67388b91fd2..1818dff1f82 100644 --- a/src/node/caches.h +++ b/src/node/caches.h @@ -5,18 +5,21 @@ #ifndef BITCOIN_NODE_CACHES_H #define BITCOIN_NODE_CACHES_H +#include + #include #include class ArgsManager; namespace node { +struct IndexCacheSizes { + int64_t tx_index{0}; + int64_t filter_index{0}; +}; struct CacheSizes { - int64_t block_tree_db; - int64_t coins_db; - int64_t coins; - int64_t tx_index; - int64_t filter_index; + IndexCacheSizes index; + kernel::CacheSizes kernel; }; CacheSizes CalculateCacheSizes(const ArgsManager& args, size_t n_indexes = 0); } // namespace node diff --git a/src/node/chainstate.cpp b/src/node/chainstate.cpp index e56896fb224..660d2c3289c 100644 --- a/src/node/chainstate.cpp +++ b/src/node/chainstate.cpp @@ -8,9 +8,9 @@ #include #include #include +#include #include #include -#include #include #include #include @@ -29,6 +29,8 @@ #include #include +using kernel::CacheSizes; + namespace node { // Complete initialization of chainstates after the initial call has been made // to ChainstateManager::InitializeChainstate(). diff --git a/src/node/chainstate.h b/src/node/chainstate.h index bb0c4f2b87e..ce414fa0434 100644 --- a/src/node/chainstate.h +++ b/src/node/chainstate.h @@ -14,9 +14,11 @@ class CTxMemPool; -namespace node { - +namespace kernel { struct CacheSizes; +} // namespace kernel + +namespace node { struct ChainstateLoadOptions { CTxMemPool* mempool{nullptr}; @@ -69,7 +71,7 @@ using ChainstateLoadResult = std::tuple; * * LoadChainstate returns a (status code, error string) tuple. */ -ChainstateLoadResult LoadChainstate(ChainstateManager& chainman, const CacheSizes& cache_sizes, +ChainstateLoadResult LoadChainstate(ChainstateManager& chainman, const kernel::CacheSizes& cache_sizes, const ChainstateLoadOptions& options); ChainstateLoadResult VerifyLoadedChainstate(ChainstateManager& chainman, const ChainstateLoadOptions& options); } // namespace node diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 12feba09a54..5ede59b82a1 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -66,7 +66,6 @@ using kernel::BlockTreeDB; using node::ApplyArgsManOptions; using node::BlockAssembler; using node::BlockManager; -using node::CalculateCacheSizes; using node::KernelNotifications; using node::LoadChainstate; using node::RegenerateCommitments; @@ -228,8 +227,6 @@ ChainTestingSetup::ChainTestingSetup(const ChainType chainType, TestOpts opts) Assert(error.empty()); m_node.warnings = std::make_unique(); - m_cache_sizes = CalculateCacheSizes(m_args); - m_node.notifications = std::make_unique(Assert(m_node.shutdown_request), m_node.exit_status, *Assert(m_node.warnings)); m_make_chainman = [this, &chainparams, opts] { @@ -255,7 +252,7 @@ ChainTestingSetup::ChainTestingSetup(const ChainType chainType, TestOpts opts) LOCK(m_node.chainman->GetMutex()); m_node.chainman->m_blockman.m_block_tree_db = std::make_unique(DBParams{ .path = m_args.GetDataDirNet() / "blocks" / "index", - .cache_bytes = static_cast(m_cache_sizes.block_tree_db), + .cache_bytes = static_cast(m_kernel_cache_sizes.block_tree_db), .memory_only = true, }); }; @@ -291,7 +288,7 @@ void ChainTestingSetup::LoadVerifyActivateChainstate() options.check_blocks = m_args.GetIntArg("-checkblocks", DEFAULT_CHECKBLOCKS); options.check_level = m_args.GetIntArg("-checklevel", DEFAULT_CHECKLEVEL); options.require_full_verification = m_args.IsArgSet("-checkblocks") || m_args.IsArgSet("-checklevel"); - auto [status, error] = LoadChainstate(chainman, m_cache_sizes, options); + auto [status, error] = LoadChainstate(chainman, m_kernel_cache_sizes, options); assert(status == node::ChainstateLoadStatus::SUCCESS); std::tie(status, error) = VerifyLoadedChainstate(chainman, options); diff --git a/src/test/util/setup_common.h b/src/test/util/setup_common.h index b0f7bdade20..33ad2584573 100644 --- a/src/test/util/setup_common.h +++ b/src/test/util/setup_common.h @@ -6,6 +6,7 @@ #define BITCOIN_TEST_UTIL_SETUP_COMMON_H #include // IWYU pragma: export +#include #include #include #include @@ -103,7 +104,7 @@ struct BasicTestingSetup { * initialization behaviour. */ struct ChainTestingSetup : public BasicTestingSetup { - node::CacheSizes m_cache_sizes{}; + kernel::CacheSizes m_kernel_cache_sizes{node::CalculateCacheSizes(m_args).kernel}; bool m_coins_db_in_memory{true}; bool m_block_tree_db_in_memory{true}; std::function m_make_chainman{}; diff --git a/src/txdb.h b/src/txdb.h index 671cbd12866..57ae5267135 100644 --- a/src/txdb.h +++ b/src/txdb.h @@ -27,16 +27,12 @@ static const int64_t nDefaultDbCache = 450; static const int64_t nDefaultDbBatchSize = 16 << 20; //! min. -dbcache (MiB) static const int64_t nMinDbCache = 4; -//! Max memory allocated to block tree DB specific cache (MiB) -static const int64_t nMaxBlockDBCache = 2; // Unlike for the UTXO database, for the txindex scenario the leveldb cache make // a meaningful difference: https://github.com/bitcoin/bitcoin/pull/8273#issuecomment-229601991 //! Max memory allocated to tx index DB specific cache in MiB. static const int64_t nMaxTxIndexCache = 1024; //! Max memory allocated to all block filter index caches combined in MiB. static const int64_t max_filter_index_cache = 1024; -//! Max memory allocated to coin DB specific cache (MiB) -static const int64_t nMaxCoinsDBCache = 8; //! User-controlled performance and debug options. struct CoinsViewOptions { From 8826cae285490439dc1f19b25fa70b2b9e62dfe8 Mon Sep 17 00:00:00 2001 From: TheCharlatan Date: Fri, 22 Nov 2024 22:44:17 +0100 Subject: [PATCH 6/8] kernel: Move non-kernel db cache size constants These have nothing to do with the txdb, so move them out and into the node caches. --- src/node/caches.cpp | 11 +++++++++-- src/node/caches.h | 5 ++--- src/txdb.h | 6 ------ 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/node/caches.cpp b/src/node/caches.cpp index 50b8bceebe8..c1d1d819927 100644 --- a/src/node/caches.cpp +++ b/src/node/caches.cpp @@ -12,16 +12,23 @@ #include #include +// Unlike for the UTXO database, for the txindex scenario the leveldb cache make +// a meaningful difference: https://github.com/bitcoin/bitcoin/pull/8273#issuecomment-229601991 +//! Max memory allocated to tx index DB specific cache in MiB. +static constexpr int64_t MAX_TX_INDEX_CACHE{1024}; +//! Max memory allocated to all block filter index caches combined in MiB. +static constexpr int64_t MAX_FILTER_INDEX_CACHE{1024}; + namespace node { CacheSizes CalculateCacheSizes(const ArgsManager& args, size_t n_indexes) { int64_t nTotalCache = (args.GetIntArg("-dbcache", nDefaultDbCache) << 20); nTotalCache = std::max(nTotalCache, nMinDbCache << 20); // total cache cannot be less than nMinDbCache IndexCacheSizes sizes; - sizes.tx_index = std::min(nTotalCache / 8, args.GetBoolArg("-txindex", DEFAULT_TXINDEX) ? nMaxTxIndexCache << 20 : 0); + sizes.tx_index = std::min(nTotalCache / 8, args.GetBoolArg("-txindex", DEFAULT_TXINDEX) ? MAX_TX_INDEX_CACHE << 20 : 0); nTotalCache -= sizes.tx_index; if (n_indexes > 0) { - int64_t max_cache = std::min(nTotalCache / 8, max_filter_index_cache << 20); + int64_t max_cache = std::min(nTotalCache / 8, MAX_FILTER_INDEX_CACHE << 20); sizes.filter_index = max_cache / n_indexes; nTotalCache -= sizes.filter_index * n_indexes; } diff --git a/src/node/caches.h b/src/node/caches.h index 1818dff1f82..0ef5ddb7c5a 100644 --- a/src/node/caches.h +++ b/src/node/caches.h @@ -8,14 +8,13 @@ #include #include -#include class ArgsManager; namespace node { struct IndexCacheSizes { - int64_t tx_index{0}; - int64_t filter_index{0}; + size_t tx_index{0}; + size_t filter_index{0}; }; struct CacheSizes { IndexCacheSizes index; diff --git a/src/txdb.h b/src/txdb.h index 57ae5267135..ab2cb641fc9 100644 --- a/src/txdb.h +++ b/src/txdb.h @@ -27,12 +27,6 @@ static const int64_t nDefaultDbCache = 450; static const int64_t nDefaultDbBatchSize = 16 << 20; //! min. -dbcache (MiB) static const int64_t nMinDbCache = 4; -// Unlike for the UTXO database, for the txindex scenario the leveldb cache make -// a meaningful difference: https://github.com/bitcoin/bitcoin/pull/8273#issuecomment-229601991 -//! Max memory allocated to tx index DB specific cache in MiB. -static const int64_t nMaxTxIndexCache = 1024; -//! Max memory allocated to all block filter index caches combined in MiB. -static const int64_t max_filter_index_cache = 1024; //! User-controlled performance and debug options. struct CoinsViewOptions { From 65cde3621dbb9ac7d210d4926e7601c4adf5f498 Mon Sep 17 00:00:00 2001 From: TheCharlatan Date: Fri, 22 Nov 2024 22:52:02 +0100 Subject: [PATCH 7/8] kernel: Move default cache constants to caches They are not related to the txdb, so a better place for them is the new kernel and node cache file. Re-use the default amount of kernel cache for the default node cache. --- src/bitcoin-chainstate.cpp | 4 ++-- src/init.cpp | 2 +- src/kernel/caches.h | 2 ++ src/node/caches.cpp | 5 ++--- src/node/caches.h | 5 +++++ src/qt/optionsdialog.cpp | 6 +++--- src/qt/optionsmodel.cpp | 6 +++--- src/txdb.h | 4 ---- 8 files changed, 18 insertions(+), 16 deletions(-) diff --git a/src/bitcoin-chainstate.cpp b/src/bitcoin-chainstate.cpp index 3b551e5c178..50c6e0b6f17 100644 --- a/src/bitcoin-chainstate.cpp +++ b/src/bitcoin-chainstate.cpp @@ -19,9 +19,9 @@ #include #include +#include #include #include -#include #include #include #include