From db43ab9c4c0edb2fb880d3f3c1d906d55e04cf1a 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 | 35 +++++++++++++++++++++------------- src/node/caches.h | 10 +++++----- 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 50c6e0b6f17d0a..51c482607aa8f4 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 8827c41fea87cf..5c0fc9fbff1c9b 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 7343eee66aedf7..33ae604772b7d9 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 d2606dab5885ea..990089790fd3d3 100644 --- a/src/node/caches.cpp +++ b/src/node/caches.cpp @@ -7,30 +7,39 @@ #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; + // 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; + size_t clamped_db_cache = std::min(*db_cache, std::numeric_limits::max()); + total_cache = std::max(MIN_DB_CACHE, SaturatingLeftShift(clamped_db_cache, 20)); + } + + 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; + 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; + 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 {sizes, kernel::CacheSizes{static_cast(nTotalCache)}}; + return {index_sizes, kernel::CacheSizes{total_cache}}; } } // namespace node diff --git a/src/node/caches.h b/src/node/caches.h index 8b57ad9afed707..05499f3a4b52e7 100644 --- a/src/node/caches.h +++ b/src/node/caches.h @@ -6,16 +6,16 @@ #define BITCOIN_NODE_CACHES_H #include +#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 660d2c3289c659..3ee557ae62222f 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 76b7852109e780..2cea1bf6ab4d18 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 2c6f13dc6d47a4..5782a57a265857 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 5ede59b82a1807..1c7bfa85d5111e 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 aea7a4621bbd36..f75337604ad52e 100644 --- a/src/validation.h +++ b/src/validation.h @@ -1067,11 +1067,11 @@ class ChainstateManager //! 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. //!