From 5db7d4d3d28bd1269a09955b4695135c86c4827d Mon Sep 17 00:00:00 2001 From: TheCharlatan Date: Wed, 11 Dec 2024 22:15:22 +0100 Subject: [PATCH 1/6] 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 565b060dbe..671cbd1286 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 19316eccaf93776c49b1870f8aa9a5fe57ec5f33 Mon Sep 17 00:00:00 2001 From: TheCharlatan Date: Tue, 17 Dec 2024 21:33:56 +0100 Subject: [PATCH 2/6] 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. 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 dc4d98f592..01b08e320d 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 41bc7164880ffd0782ff5882211c4fcadd656022 Mon Sep 17 00:00:00 2001 From: TheCharlatan Date: Fri, 22 Nov 2024 20:51:34 +0100 Subject: [PATCH 3/6] 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) Co-authored-by: Hodlinator <172445034+hodlinator@users.noreply.github.com> --- src/bitcoin-chainstate.cpp | 5 +--- src/init.cpp | 26 +++++++++++---------- src/kernel/caches.h | 42 ++++++++++++++++++++++++++++++++++ src/node/caches.cpp | 13 +++++------ src/node/caches.h | 11 +++++---- src/node/chainstate.cpp | 4 +++- src/node/chainstate.h | 8 ++++--- src/test/util/setup_common.cpp | 7 ++---- src/test/util/setup_common.h | 3 ++- 9 files changed, 82 insertions(+), 37 deletions(-) create mode 100644 src/kernel/caches.h diff --git a/src/bitcoin-chainstate.cpp b/src/bitcoin-chainstate.cpp index bab09f13d9..c9e82eed3c 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{MiBToBytes(nDefaultDbCache)}; 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 a4ad2771cc..612b04a98e 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -32,6 +32,7 @@ #include #include #include +#include #include #include #include @@ -119,9 +120,10 @@ using common::AmountErrMsg; using common::InvalidPortErrMsg; using common::ResolveErrMsg; +using kernel::CacheSizes; + using node::ApplyArgsManOptions; using node::BlockManager; -using node::CacheSizes; using node::CalculateCacheSizes; using node::ChainstateLoadResult; using node::ChainstateLoadStatus; @@ -1603,18 +1605,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()); + 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 +1629,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 +1648,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 +1674,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 0000000000..3f1e20474d --- /dev/null +++ b/src/kernel/caches.h @@ -0,0 +1,42 @@ +// 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 + +#include +#include +#include +#include + +//! Guard against truncation of values before converting. +constexpr size_t MiBToBytes(int64_t mib) +{ + Assert(std::countl_zero(static_cast(mib)) >= 21); // Ensure sign bit is unset + enough zeros to shift. + const int64_t bytes{mib << 20}; + Assert(static_cast(bytes) <= std::numeric_limits::max()); + return static_cast(bytes); +} + +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, MiBToBytes(nMaxBlockDBCache)); + total_cache -= block_tree_db; + coins_db = std::min(total_cache / 2, MiBToBytes(nMaxCoinsDBCache)); + 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 01b08e320d..d930f38605 100644 --- a/src/node/caches.cpp +++ b/src/node/caches.cpp @@ -6,16 +6,18 @@ #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; @@ -24,9 +26,6 @@ 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, 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 67388b91fd..3d8679553a 100644 --- a/src/node/caches.h +++ b/src/node/caches.h @@ -5,19 +5,22 @@ #ifndef BITCOIN_NODE_CACHES_H #define BITCOIN_NODE_CACHES_H +#include + #include #include class ArgsManager; namespace node { -struct CacheSizes { - int64_t block_tree_db; - int64_t coins_db; - int64_t coins; +struct IndexCacheSizes { int64_t tx_index; int64_t filter_index; }; +struct CacheSizes { + 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 e56896fb22..660d2c3289 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 bb0c4f2b87..ce414fa043 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 12feba09a5..5ede59b82a 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 b0f7bdade2..33ad258457 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{}; From 136e4080c22eeb3b5b67ddb81b1cd814156a8190 Mon Sep 17 00:00:00 2001 From: TheCharlatan Date: Fri, 22 Nov 2024 22:44:17 +0100 Subject: [PATCH 4/6] 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/txdb.h | 6 ------ 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/node/caches.cpp b/src/node/caches.cpp index d930f38605..a771af46fc 100644 --- a/src/node/caches.cpp +++ b/src/node/caches.cpp @@ -12,17 +12,24 @@ #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; sizes.filter_index = 0; 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/txdb.h b/src/txdb.h index 671cbd1286..a92f19631c 100644 --- a/src/txdb.h +++ b/src/txdb.h @@ -29,12 +29,6 @@ static const int64_t nDefaultDbBatchSize = 16 << 20; 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; From 1f81be60f5f69d520a708e7808e56aa086b48271 Mon Sep 17 00:00:00 2001 From: TheCharlatan Date: Fri, 22 Nov 2024 22:52:02 +0100 Subject: [PATCH 5/6] kernel: Move kernel-related cache constants to kernel cache They are not all related to the txdb, so a better place for them is the new kernel cache file. --- src/bitcoin-chainstate.cpp | 4 ++-- src/init.cpp | 2 +- src/kernel/caches.h | 12 +++++++++--- src/node/caches.cpp | 5 ++--- src/node/caches.h | 5 +++++ src/qt/optionsdialog.cpp | 6 +++--- src/qt/optionsmodel.cpp | 6 +++--- src/txdb.h | 8 -------- 8 files changed, 25 insertions(+), 23 deletions(-) diff --git a/src/bitcoin-chainstate.cpp b/src/bitcoin-chainstate.cpp index c9e82eed3c..46ca35ea90 100644 --- a/src/bitcoin-chainstate.cpp +++ b/src/bitcoin-chainstate.cpp @@ -19,9 +19,9 @@ #include #include +#include #include #include -#include #include #include #include