mirror of
https://github.com/bitcoin/bitcoin.git
synced 2026-05-29 23:33:33 +02:00
Merge bitcoin/bitcoin#34435: refactor: use _MiB/_GiB consistently for byte conversions
af0ee28eb6refactor: use _MiB consistently for Mebibyte conversions (Lőrinc)b3edd30aa2util: add _GiB for Gibibyte conversions (Lőrinc) Pull request description: ### Problem Byte-size conversions in the codebase currently show up in many equivalent formats (multiplication/division chains, shifts, hex/binary literals), which creates a maintenance burden and makes review error-prone - especially considering the architectural differences of `size_t`. Inspired by https://github.com/bitcoin/bitcoin/pull/34305#discussion_r2734720002, it seemed appropriate to unify `Mebibyte` usage across the codebase and add `Gibibyte` support with 32/64 bit `size_t` validation. ### Fix This PR refactors those call sites to use `""_MiB` (existing) and `""_GiB` (new), and adds the encountered value/pattern replacements to unit tests to make review straightforward, and to ensure the conversions remain valid. The literals are overflow-checked when converting to `size_t`, and unit tests cover the 32-bit boundary cases. Concretely, it replaces patterns such as: * `1024*1024`, `1<<20`, `0x100000`, `1048576`, `/ 1024 / 1024`, `* (1.0 / 1024 / 1024)` → `1_MiB` or `double(1_MiB)` * `1024*1024*1024`, `1<<30`, `0x40000000`, `1024_MiB`, `>> 30` → `1_GiB` (added unit tests for each replacement category to ease review) Additionally, declarations whose initializer reads a `_MiB`/`_GiB` literal are switched to braced initialization so a future oversized value is rejected at compile time through the narrowing check instead of silently truncating. ### Note In the few places where arithmetic involves signed values, the result is identical to the previous code assuming those quantities never become negative. ACKs for top commit: achow101: ACKaf0ee28eb6janb84: ACKaf0ee28eb6maflcko: review ACKaf0ee28eb6🖍 hodlinator: re-ACKaf0ee28eb6Tree-SHA512: 55286ce3f833f88335394a74e9e0b95c7d023e5cdc9ded40accbbbcd870101e4dcc05926865d6bef4c1be1ebd648aa3fdf947ef9575633ccfe56691f145d7a2d
This commit is contained in:
@@ -8,6 +8,7 @@
|
||||
#include <node/blockstorage.h>
|
||||
#include <node/database_args.h>
|
||||
#include <tinyformat.h>
|
||||
#include <util/byte_units.h>
|
||||
#include <util/result.h>
|
||||
#include <util/translation.h>
|
||||
#include <validation.h>
|
||||
@@ -23,12 +24,12 @@ util::Result<void> ApplyArgsManOptions(const ArgsManager& args, BlockManager::Op
|
||||
if (nPruneArg < 0) {
|
||||
return util::Error{_("Prune cannot be configured with a negative value.")};
|
||||
}
|
||||
uint64_t nPruneTarget{uint64_t(nPruneArg) * 1024 * 1024};
|
||||
uint64_t nPruneTarget{uint64_t(nPruneArg) * 1_MiB};
|
||||
if (nPruneArg == 1) { // manual pruning: -prune=1
|
||||
nPruneTarget = BlockManager::PRUNE_TARGET_MANUAL;
|
||||
} else if (nPruneTarget) {
|
||||
if (nPruneTarget < MIN_DISK_SPACE_FOR_BLOCK_FILES) {
|
||||
return util::Error{strprintf(_("Prune configured below the minimum of %d MiB. Please use a higher number."), MIN_DISK_SPACE_FOR_BLOCK_FILES / 1024 / 1024)};
|
||||
return util::Error{strprintf(_("Prune configured below the minimum of %d MiB. Please use a higher number."), MIN_DISK_SPACE_FOR_BLOCK_FILES / 1_MiB)};
|
||||
}
|
||||
}
|
||||
opts.prune_target = nPruneTarget;
|
||||
|
||||
@@ -394,8 +394,8 @@ void BlockManager::FindFilesToPrune(
|
||||
}
|
||||
|
||||
LogDebug(BCLog::PRUNE, "[%s] target=%dMiB actual=%dMiB diff=%dMiB min_height=%d max_prune_height=%d removed %d blk/rev pairs\n",
|
||||
chain.GetRole(), target / 1024 / 1024, nCurrentUsage / 1024 / 1024,
|
||||
(int64_t(target) - int64_t(nCurrentUsage)) / 1024 / 1024,
|
||||
chain.GetRole(), target / 1_MiB, nCurrentUsage / 1_MiB,
|
||||
(int64_t(target) - int64_t(nCurrentUsage)) / int64_t(1_MiB),
|
||||
min_block_to_prune, last_block_can_prune, count);
|
||||
}
|
||||
|
||||
|
||||
@@ -18,6 +18,7 @@
|
||||
#include <streams.h>
|
||||
#include <sync.h>
|
||||
#include <uint256.h>
|
||||
#include <util/byte_units.h> // IWYU pragma: keep
|
||||
#include <util/expected.h>
|
||||
#include <util/fs.h>
|
||||
#include <util/hasher.h>
|
||||
@@ -116,11 +117,11 @@ using kernel::CBlockFileInfo;
|
||||
using kernel::BlockTreeDB;
|
||||
|
||||
/** The pre-allocation chunk size for blk?????.dat files (since 0.8) */
|
||||
static const unsigned int BLOCKFILE_CHUNK_SIZE = 0x1000000; // 16 MiB
|
||||
static const unsigned int BLOCKFILE_CHUNK_SIZE{16_MiB};
|
||||
/** The pre-allocation chunk size for rev?????.dat files (since 0.8) */
|
||||
static const unsigned int UNDOFILE_CHUNK_SIZE = 0x100000; // 1 MiB
|
||||
static const unsigned int UNDOFILE_CHUNK_SIZE{1_MiB};
|
||||
/** The maximum size of a blk?????.dat file (since 0.8) */
|
||||
static const unsigned int MAX_BLOCKFILE_SIZE = 0x8000000; // 128 MiB
|
||||
static const unsigned int MAX_BLOCKFILE_SIZE{128_MiB};
|
||||
|
||||
/** Size of header written by WriteBlock before a serialized CBlock (8 bytes) */
|
||||
static constexpr uint32_t STORAGE_HEADER_BYTES{std::tuple_size_v<MessageStartChars> + sizeof(unsigned int)};
|
||||
|
||||
@@ -20,17 +20,17 @@
|
||||
// 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 bytes.
|
||||
static constexpr size_t MAX_TX_INDEX_CACHE{1024_MiB};
|
||||
static constexpr size_t MAX_TX_INDEX_CACHE{1_GiB};
|
||||
//! Max memory allocated to all block filter index caches combined in bytes.
|
||||
static constexpr size_t MAX_FILTER_INDEX_CACHE{1024_MiB};
|
||||
static constexpr size_t MAX_FILTER_INDEX_CACHE{1_GiB};
|
||||
//! Max memory allocated to tx spenderindex DB specific cache in bytes.
|
||||
static constexpr size_t MAX_TXOSPENDER_INDEX_CACHE{1024_MiB};
|
||||
static constexpr size_t MAX_TXOSPENDER_INDEX_CACHE{1_GiB};
|
||||
//! Maximum dbcache size on 32-bit systems.
|
||||
static constexpr size_t MAX_32BIT_DBCACHE{1024_MiB};
|
||||
static constexpr size_t MAX_32BIT_DBCACHE{1_GiB};
|
||||
//! Larger default dbcache on 64-bit systems with enough RAM.
|
||||
static constexpr size_t HIGH_DEFAULT_DBCACHE{1024_MiB};
|
||||
static constexpr size_t HIGH_DEFAULT_DBCACHE{1_GiB};
|
||||
//! Minimum detected RAM required for HIGH_DEFAULT_DBCACHE.
|
||||
static constexpr uint64_t HIGH_DEFAULT_DBCACHE_MIN_TOTAL_RAM{4096ULL << 20};
|
||||
static constexpr uint64_t HIGH_DEFAULT_DBCACHE_MIN_TOTAL_RAM{uint64_t{4} * 1_GiB};
|
||||
|
||||
namespace node {
|
||||
size_t GetDefaultDBCache()
|
||||
|
||||
@@ -31,7 +31,7 @@ struct CacheSizes {
|
||||
CacheSizes CalculateCacheSizes(const ArgsManager& args, size_t n_indexes = 0);
|
||||
constexpr bool ShouldWarnOversizedDbCache(size_t dbcache, size_t total_ram) noexcept
|
||||
{
|
||||
const size_t cap{(total_ram < 2048_MiB) ? DEFAULT_DB_CACHE : (total_ram / 100) * 75};
|
||||
const size_t cap{(total_ram < 2_GiB) ? DEFAULT_DB_CACHE : (total_ram / 100) * 75};
|
||||
return dbcache > cap;
|
||||
}
|
||||
|
||||
|
||||
@@ -14,6 +14,7 @@
|
||||
#include <tinyformat.h>
|
||||
#include <txdb.h>
|
||||
#include <uint256.h>
|
||||
#include <util/byte_units.h>
|
||||
#include <util/fs.h>
|
||||
#include <util/log.h>
|
||||
#include <util/signalinterrupt.h>
|
||||
@@ -163,7 +164,7 @@ ChainstateLoadResult LoadChainstate(ChainstateManager& chainman, const CacheSize
|
||||
LogInfo("Block pruning enabled. Use RPC call pruneblockchain(height) to manually prune block and undo files.");
|
||||
} else if (chainman.m_blockman.GetPruneTarget()) {
|
||||
LogInfo("Prune configured to target %u MiB on disk for block and undo files.",
|
||||
chainman.m_blockman.GetPruneTarget() / 1024 / 1024);
|
||||
chainman.m_blockman.GetPruneTarget() / 1_MiB);
|
||||
}
|
||||
|
||||
LOCK(cs_main);
|
||||
|
||||
@@ -12,6 +12,7 @@
|
||||
#include <node/database_args.h>
|
||||
#include <tinyformat.h>
|
||||
#include <uint256.h>
|
||||
#include <util/byte_units.h>
|
||||
#include <util/result.h>
|
||||
#include <util/strencodings.h>
|
||||
#include <util/translation.h>
|
||||
@@ -64,7 +65,7 @@ util::Result<void> ApplyArgsManOptions(const ArgsManager& args, ChainstateManage
|
||||
// script execution cache create the minimum possible cache (2
|
||||
// elements). Therefore, we can use 0 as a floor here.
|
||||
// 2. Multiply first, divide after to avoid integer truncation.
|
||||
size_t clamped_size_each = std::max<int64_t>(*max_size, 0) * (1 << 20) / 2;
|
||||
size_t clamped_size_each{size_t(std::max<int64_t>(*max_size, 0) * 1_MiB / 2)};
|
||||
opts.script_execution_cache_bytes = clamped_size_each;
|
||||
opts.signature_cache_bytes = clamped_size_each;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user