From 2a92702bafca5c78b270a9502a22cb9deac02cfc Mon Sep 17 00:00:00 2001 From: TheCharlatan Date: Tue, 17 Dec 2024 21:58:47 +0100 Subject: [PATCH] init: Use size_t consistently for cache sizes This avoids having to rely on implicit casts when passing them to the various functions allocating the caches. This also ensures that if the requested amount of db_cache does not fit in a size_t, it is clamped to the maximum value of a size_t. Also take this opportunity to make the total amounts of cache in the chainstate manager a size_t too. --- src/bitcoin-chainstate.cpp | 2 +- src/init.cpp | 2 +- src/kernel/caches.h | 4 ++-- src/node/caches.cpp | 36 +++++++++++++++++++++------------- src/node/caches.h | 9 +++++---- src/node/chainstate.cpp | 2 +- src/qt/optionsdialog.cpp | 2 +- src/qt/optionsmodel.cpp | 4 ++-- src/test/util/setup_common.cpp | 2 +- src/validation.h | 4 ++-- 10 files changed, 38 insertions(+), 29 deletions(-) diff --git a/src/bitcoin-chainstate.cpp b/src/bitcoin-chainstate.cpp index 50c6e0b6f17..51c482607aa 100644 --- a/src/bitcoin-chainstate.cpp +++ b/src/bitcoin-chainstate.cpp @@ -123,7 +123,7 @@ int main(int argc, char* argv[]) util::SignalInterrupt interrupt; ChainstateManager chainman{interrupt, chainman_opts, blockman_opts}; - kernel::CacheSizes cache_sizes{DEFAULT_KERNEL_CACHE << 20}; + kernel::CacheSizes cache_sizes{DEFAULT_KERNEL_CACHE}; 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 8827c41fea8..5c0fc9fbff1 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -487,7 +487,7 @@ void SetupServerArgs(ArgsManager& argsman, bool can_listen_ipc) argsman.AddArg("-conf=", strprintf("Specify path to read-only configuration file. Relative paths will be prefixed by datadir location (only useable from command line, not configuration file) (default: %s)", BITCOIN_CONF_FILENAME), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); argsman.AddArg("-datadir=", "Specify data directory", ArgsManager::ALLOW_ANY | ArgsManager::DISALLOW_NEGATION, OptionsCategory::OPTIONS); argsman.AddArg("-dbbatchsize", strprintf("Maximum database write batch size in bytes (default: %u)", nDefaultDbBatchSize), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::OPTIONS); - argsman.AddArg("-dbcache=", strprintf("Maximum database cache size MiB (minimum %d, default: %d). Make sure you have enough RAM. In addition, unused memory allocated to the mempool is shared with this cache (see -maxmempool).", MIN_DB_CACHE, DEFAULT_DB_CACHE), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); + argsman.AddArg("-dbcache=", strprintf("Maximum database cache size MiB (minimum %d, default: %d). Make sure you have enough RAM. In addition, unused memory allocated to the mempool is shared with this cache (see -maxmempool).", MIN_DB_CACHE >> 20, DEFAULT_DB_CACHE >> 20), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); argsman.AddArg("-includeconf=", "Specify additional configuration file, relative to the -datadir path (only useable from configuration file, not command line)", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); argsman.AddArg("-allowignoredconf", strprintf("For backwards compatibility, treat an unused %s file in the datadir as a warning, not an error.", BITCOIN_CONF_FILENAME), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); argsman.AddArg("-loadblock=", "Imports blocks from external file on startup", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); diff --git a/src/kernel/caches.h b/src/kernel/caches.h index 7343eee66ae..33ae604772b 100644 --- a/src/kernel/caches.h +++ b/src/kernel/caches.h @@ -9,8 +9,8 @@ #include -//! Suggested default amount of cache reserved for the kernel (MiB) -static constexpr int64_t DEFAULT_KERNEL_CACHE{450}; +//! Suggested default amount of cache reserved for the kernel (bytes) +static constexpr size_t DEFAULT_KERNEL_CACHE{450_MiB}; //! 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) diff --git a/src/node/caches.cpp b/src/node/caches.cpp index d2606dab588..8b432637c73 100644 --- a/src/node/caches.cpp +++ b/src/node/caches.cpp @@ -7,30 +7,38 @@ #include #include #include +#include +#include #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}; +//! Max memory allocated to tx index DB specific cache in bytes. +static constexpr size_t MAX_TX_INDEX_CACHE{1024_MiB}; +//! Max memory allocated to all block filter index caches combined in bytes. +static constexpr size_t MAX_FILTER_INDEX_CACHE{1024_MiB}; namespace node { CacheSizes CalculateCacheSizes(const ArgsManager& args, size_t n_indexes) { - int64_t nTotalCache = (args.GetIntArg("-dbcache", DEFAULT_DB_CACHE) << 20); - nTotalCache = std::max(nTotalCache, MIN_DB_CACHE << 20); - IndexCacheSizes sizes; - 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); - sizes.filter_index = max_cache / n_indexes; - nTotalCache -= sizes.filter_index * n_indexes; + // Convert -dbcache from MiB units to bytes. The total cache is floored by MIN_DB_CACHE and capped by max size_t value. + size_t total_cache{DEFAULT_DB_CACHE}; + if (std::optional db_cache = args.GetIntArg("-dbcache")) { + if (*db_cache < 0) db_cache = 0; + uint64_t db_cache_bytes = SaturatingLeftShift(*db_cache, 20); + total_cache = std::max(MIN_DB_CACHE, std::min(db_cache_bytes, std::numeric_limits::max())); } - return {sizes, kernel::CacheSizes{static_cast(nTotalCache)}}; + + IndexCacheSizes index_sizes; + index_sizes.tx_index = std::min(total_cache / 8, args.GetBoolArg("-txindex", DEFAULT_TXINDEX) ? MAX_TX_INDEX_CACHE : 0); + total_cache -= index_sizes.tx_index; + if (n_indexes > 0) { + size_t max_cache = std::min(total_cache / 8, MAX_FILTER_INDEX_CACHE); + index_sizes.filter_index = max_cache / n_indexes; + total_cache -= index_sizes.filter_index * n_indexes; + } + return {index_sizes, kernel::CacheSizes{total_cache}}; } } // namespace node diff --git a/src/node/caches.h b/src/node/caches.h index d79c3889d34..f24e9cc9103 100644 --- a/src/node/caches.h +++ b/src/node/caches.h @@ -6,15 +6,16 @@ #define BITCOIN_NODE_CACHES_H #include +#include #include class ArgsManager; -//! min. -dbcache (MiB) -static constexpr int64_t MIN_DB_CACHE{4}; -//! -dbcache default (MiB) -static constexpr int64_t DEFAULT_DB_CACHE{DEFAULT_KERNEL_CACHE}; +//! min. -dbcache (bytes) +static constexpr size_t MIN_DB_CACHE{4_MiB}; +//! -dbcache default (bytes) +static constexpr size_t DEFAULT_DB_CACHE{DEFAULT_KERNEL_CACHE}; namespace node { struct IndexCacheSizes { diff --git a/src/node/chainstate.cpp b/src/node/chainstate.cpp index 660d2c3289c..3ee557ae622 100644 --- a/src/node/chainstate.cpp +++ b/src/node/chainstate.cpp @@ -46,7 +46,7 @@ static ChainstateLoadResult CompleteChainstateInitialization( try { pblocktree = std::make_unique(DBParams{ .path = chainman.m_options.datadir / "blocks" / "index", - .cache_bytes = static_cast(cache_sizes.block_tree_db), + .cache_bytes = cache_sizes.block_tree_db, .memory_only = options.block_tree_db_in_memory, .wipe_data = options.wipe_block_tree_db, .options = chainman.m_options.block_tree_db}); diff --git a/src/qt/optionsdialog.cpp b/src/qt/optionsdialog.cpp index 76b7852109e..2cea1bf6ab4 100644 --- a/src/qt/optionsdialog.cpp +++ b/src/qt/optionsdialog.cpp @@ -95,7 +95,7 @@ OptionsDialog::OptionsDialog(QWidget* parent, bool enableWallet) ui->verticalLayout->setStretchFactor(ui->tabWidget, 1); /* Main elements init */ - ui->databaseCache->setRange(MIN_DB_CACHE, std::numeric_limits::max()); + ui->databaseCache->setRange(MIN_DB_CACHE >> 20, std::numeric_limits::max()); ui->threadsScriptVerif->setMinimum(-GetNumCores()); ui->threadsScriptVerif->setMaximum(MAX_SCRIPTCHECK_THREADS); ui->pruneWarning->setVisible(false); diff --git a/src/qt/optionsmodel.cpp b/src/qt/optionsmodel.cpp index 2c6f13dc6d4..5782a57a265 100644 --- a/src/qt/optionsmodel.cpp +++ b/src/qt/optionsmodel.cpp @@ -470,7 +470,7 @@ QVariant OptionsModel::getOption(OptionID option, const std::string& suffix) con suffix.empty() ? getOption(option, "-prev") : DEFAULT_PRUNE_TARGET_GB; case DatabaseCache: - return qlonglong(SettingToInt(setting(), DEFAULT_DB_CACHE)); + return qlonglong(SettingToInt(setting(), DEFAULT_DB_CACHE >> 20)); case ThreadsScriptVerif: return qlonglong(SettingToInt(setting(), DEFAULT_SCRIPTCHECK_THREADS)); case Listen: @@ -733,7 +733,7 @@ void OptionsModel::checkAndMigrate() // see https://github.com/bitcoin/bitcoin/pull/8273 // force people to upgrade to the new value if they are using 100MB if (settingsVersion < 130000 && settings.contains("nDatabaseCache") && settings.value("nDatabaseCache").toLongLong() == 100) - settings.setValue("nDatabaseCache", (qint64)DEFAULT_DB_CACHE); + settings.setValue("nDatabaseCache", (qint64)(DEFAULT_DB_CACHE >> 20)); settings.setValue(strSettingsVersionKey, CLIENT_VERSION); } diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 5ede59b82a1..1c7bfa85d51 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -252,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_kernel_cache_sizes.block_tree_db), + .cache_bytes = m_kernel_cache_sizes.block_tree_db, .memory_only = true, }); }; diff --git a/src/validation.h b/src/validation.h index aea7a4621bb..f75337604ad 100644 --- a/src/validation.h +++ b/src/validation.h @@ -1067,11 +1067,11 @@ public: //! The total number of bytes available for us to use across all in-memory //! coins caches. This will be split somehow across chainstates. - int64_t m_total_coinstip_cache{0}; + size_t m_total_coinstip_cache{0}; // //! The total number of bytes available for us to use across all leveldb //! coins databases. This will be split somehow across chainstates. - int64_t m_total_coinsdb_cache{0}; + size_t m_total_coinsdb_cache{0}; //! Instantiate a new chainstate. //!