Skip to content

Commit 2a92702

Browse files
committed
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.
1 parent 65cde36 commit 2a92702

10 files changed

+37
-28
lines changed

Diff for: src/bitcoin-chainstate.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ int main(int argc, char* argv[])
123123
util::SignalInterrupt interrupt;
124124
ChainstateManager chainman{interrupt, chainman_opts, blockman_opts};
125125

126-
kernel::CacheSizes cache_sizes{DEFAULT_KERNEL_CACHE << 20};
126+
kernel::CacheSizes cache_sizes{DEFAULT_KERNEL_CACHE};
127127
node::ChainstateLoadOptions options;
128128
auto [status, error] = node::LoadChainstate(chainman, cache_sizes, options);
129129
if (status != node::ChainstateLoadStatus::SUCCESS) {

Diff for: src/init.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -487,7 +487,7 @@ void SetupServerArgs(ArgsManager& argsman, bool can_listen_ipc)
487487
argsman.AddArg("-conf=<file>", 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);
488488
argsman.AddArg("-datadir=<dir>", "Specify data directory", ArgsManager::ALLOW_ANY | ArgsManager::DISALLOW_NEGATION, OptionsCategory::OPTIONS);
489489
argsman.AddArg("-dbbatchsize", strprintf("Maximum database write batch size in bytes (default: %u)", nDefaultDbBatchSize), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::OPTIONS);
490-
argsman.AddArg("-dbcache=<n>", strprintf("Maximum database cache size <n> 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);
490+
argsman.AddArg("-dbcache=<n>", strprintf("Maximum database cache size <n> 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);
491491
argsman.AddArg("-includeconf=<file>", "Specify additional configuration file, relative to the -datadir path (only useable from configuration file, not command line)", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
492492
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);
493493
argsman.AddArg("-loadblock=<file>", "Imports blocks from external file on startup", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);

Diff for: src/kernel/caches.h

+2-2
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@
99

1010
#include <algorithm>
1111

12-
//! Suggested default amount of cache reserved for the kernel (MiB)
13-
static constexpr int64_t DEFAULT_KERNEL_CACHE{450};
12+
//! Suggested default amount of cache reserved for the kernel (bytes)
13+
static constexpr size_t DEFAULT_KERNEL_CACHE{450_MiB};
1414
//! Max memory allocated to block tree DB specific cache (bytes)
1515
static constexpr size_t MAX_BLOCK_DB_CACHE{2_MiB};
1616
//! Max memory allocated to coin DB specific cache (bytes)

Diff for: src/node/caches.cpp

+21-13
Original file line numberDiff line numberDiff line change
@@ -7,30 +7,38 @@
77
#include <common/args.h>
88
#include <index/txindex.h>
99
#include <kernel/caches.h>
10+
#include <logging.h>
11+
#include <util/byte_units.h>
1012

1113
#include <algorithm>
1214
#include <string>
1315

1416
// Unlike for the UTXO database, for the txindex scenario the leveldb cache make
1517
// a meaningful difference: https://github.com/bitcoin/bitcoin/pull/8273#issuecomment-229601991
16-
//! Max memory allocated to tx index DB specific cache in MiB.
17-
static constexpr int64_t MAX_TX_INDEX_CACHE{1024};
18-
//! Max memory allocated to all block filter index caches combined in MiB.
19-
static constexpr int64_t MAX_FILTER_INDEX_CACHE{1024};
18+
//! Max memory allocated to tx index DB specific cache in bytes.
19+
static constexpr size_t MAX_TX_INDEX_CACHE{1024_MiB};
20+
//! Max memory allocated to all block filter index caches combined in bytes.
21+
static constexpr size_t MAX_FILTER_INDEX_CACHE{1024_MiB};
2022

2123
namespace node {
2224
CacheSizes CalculateCacheSizes(const ArgsManager& args, size_t n_indexes)
2325
{
24-
int64_t nTotalCache = (args.GetIntArg("-dbcache", DEFAULT_DB_CACHE) << 20);
25-
nTotalCache = std::max(nTotalCache, MIN_DB_CACHE << 20);
26-
IndexCacheSizes sizes;
27-
sizes.tx_index = std::min(nTotalCache / 8, args.GetBoolArg("-txindex", DEFAULT_TXINDEX) ? MAX_TX_INDEX_CACHE << 20 : 0);
28-
nTotalCache -= sizes.tx_index;
26+
// Convert -dbcache from MiB units to bytes. The total cache is floored by MIN_DB_CACHE and capped by max size_t value.
27+
size_t total_cache{DEFAULT_DB_CACHE};
28+
if (std::optional<int64_t> db_cache = args.GetIntArg("-dbcache")) {
29+
if (*db_cache < 0) db_cache = 0;
30+
uint64_t db_cache_bytes = SaturatingLeftShift<uint64_t>(*db_cache, 20);
31+
total_cache = std::max<size_t>(MIN_DB_CACHE, std::min<uint64_t>(db_cache_bytes, std::numeric_limits<size_t>::max()));
32+
}
33+
34+
IndexCacheSizes index_sizes;
35+
index_sizes.tx_index = std::min(total_cache / 8, args.GetBoolArg("-txindex", DEFAULT_TXINDEX) ? MAX_TX_INDEX_CACHE : 0);
36+
total_cache -= index_sizes.tx_index;
2937
if (n_indexes > 0) {
30-
int64_t max_cache = std::min(nTotalCache / 8, MAX_FILTER_INDEX_CACHE << 20);
31-
sizes.filter_index = max_cache / n_indexes;
32-
nTotalCache -= sizes.filter_index * n_indexes;
38+
size_t max_cache = std::min(total_cache / 8, MAX_FILTER_INDEX_CACHE);
39+
index_sizes.filter_index = max_cache / n_indexes;
40+
total_cache -= index_sizes.filter_index * n_indexes;
3341
}
34-
return {sizes, kernel::CacheSizes{static_cast<size_t>(nTotalCache)}};
42+
return {index_sizes, kernel::CacheSizes{total_cache}};
3543
}
3644
} // namespace node

Diff for: src/node/caches.h

+5-4
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,16 @@
66
#define BITCOIN_NODE_CACHES_H
77

88
#include <kernel/caches.h>
9+
#include <util/byte_units.h>
910

1011
#include <cstddef>
1112

1213
class ArgsManager;
1314

14-
//! min. -dbcache (MiB)
15-
static constexpr int64_t MIN_DB_CACHE{4};
16-
//! -dbcache default (MiB)
17-
static constexpr int64_t DEFAULT_DB_CACHE{DEFAULT_KERNEL_CACHE};
15+
//! min. -dbcache (bytes)
16+
static constexpr size_t MIN_DB_CACHE{4_MiB};
17+
//! -dbcache default (bytes)
18+
static constexpr size_t DEFAULT_DB_CACHE{DEFAULT_KERNEL_CACHE};
1819

1920
namespace node {
2021
struct IndexCacheSizes {

Diff for: src/node/chainstate.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ static ChainstateLoadResult CompleteChainstateInitialization(
4646
try {
4747
pblocktree = std::make_unique<BlockTreeDB>(DBParams{
4848
.path = chainman.m_options.datadir / "blocks" / "index",
49-
.cache_bytes = static_cast<size_t>(cache_sizes.block_tree_db),
49+
.cache_bytes = cache_sizes.block_tree_db,
5050
.memory_only = options.block_tree_db_in_memory,
5151
.wipe_data = options.wipe_block_tree_db,
5252
.options = chainman.m_options.block_tree_db});

Diff for: src/qt/optionsdialog.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ OptionsDialog::OptionsDialog(QWidget* parent, bool enableWallet)
9595
ui->verticalLayout->setStretchFactor(ui->tabWidget, 1);
9696

9797
/* Main elements init */
98-
ui->databaseCache->setRange(MIN_DB_CACHE, std::numeric_limits<int>::max());
98+
ui->databaseCache->setRange(MIN_DB_CACHE >> 20, std::numeric_limits<int>::max());
9999
ui->threadsScriptVerif->setMinimum(-GetNumCores());
100100
ui->threadsScriptVerif->setMaximum(MAX_SCRIPTCHECK_THREADS);
101101
ui->pruneWarning->setVisible(false);

Diff for: src/qt/optionsmodel.cpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -470,7 +470,7 @@ QVariant OptionsModel::getOption(OptionID option, const std::string& suffix) con
470470
suffix.empty() ? getOption(option, "-prev") :
471471
DEFAULT_PRUNE_TARGET_GB;
472472
case DatabaseCache:
473-
return qlonglong(SettingToInt(setting(), DEFAULT_DB_CACHE));
473+
return qlonglong(SettingToInt(setting(), DEFAULT_DB_CACHE >> 20));
474474
case ThreadsScriptVerif:
475475
return qlonglong(SettingToInt(setting(), DEFAULT_SCRIPTCHECK_THREADS));
476476
case Listen:
@@ -733,7 +733,7 @@ void OptionsModel::checkAndMigrate()
733733
// see https://github.com/bitcoin/bitcoin/pull/8273
734734
// force people to upgrade to the new value if they are using 100MB
735735
if (settingsVersion < 130000 && settings.contains("nDatabaseCache") && settings.value("nDatabaseCache").toLongLong() == 100)
736-
settings.setValue("nDatabaseCache", (qint64)DEFAULT_DB_CACHE);
736+
settings.setValue("nDatabaseCache", (qint64)(DEFAULT_DB_CACHE >> 20));
737737

738738
settings.setValue(strSettingsVersionKey, CLIENT_VERSION);
739739
}

Diff for: src/test/util/setup_common.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -252,7 +252,7 @@ ChainTestingSetup::ChainTestingSetup(const ChainType chainType, TestOpts opts)
252252
LOCK(m_node.chainman->GetMutex());
253253
m_node.chainman->m_blockman.m_block_tree_db = std::make_unique<BlockTreeDB>(DBParams{
254254
.path = m_args.GetDataDirNet() / "blocks" / "index",
255-
.cache_bytes = static_cast<size_t>(m_kernel_cache_sizes.block_tree_db),
255+
.cache_bytes = m_kernel_cache_sizes.block_tree_db,
256256
.memory_only = true,
257257
});
258258
};

Diff for: src/validation.h

+2-2
Original file line numberDiff line numberDiff line change
@@ -1067,11 +1067,11 @@ class ChainstateManager
10671067

10681068
//! The total number of bytes available for us to use across all in-memory
10691069
//! coins caches. This will be split somehow across chainstates.
1070-
int64_t m_total_coinstip_cache{0};
1070+
size_t m_total_coinstip_cache{0};
10711071
//
10721072
//! The total number of bytes available for us to use across all leveldb
10731073
//! coins databases. This will be split somehow across chainstates.
1074-
int64_t m_total_coinsdb_cache{0};
1074+
size_t m_total_coinsdb_cache{0};
10751075

10761076
//! Instantiate a new chainstate.
10771077
//!

0 commit comments

Comments
 (0)