Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Correct units for "-dbcache" and "-prune" #15163

Merged
merged 1 commit into from Jan 30, 2019

Conversation

Projects
None yet
7 participants
@hebasto
Copy link
Member

commented Jan 14, 2019

Actually, all dbcache-related values in the code are measured in MiB (not in megabytes, MB) or in bytes (e.g., nTotalCache).

See: https://github.com/bitcoin/bitcoin/blob/master/src/txdb.h

bitcoin/src/init.cpp

Lines 1405 to 1424 in ba8c8b2

// cache size calculations
int64_t nTotalCache = (gArgs.GetArg("-dbcache", nDefaultDbCache) << 20);
nTotalCache = std::max(nTotalCache, nMinDbCache << 20); // total cache cannot be less than nMinDbCache
nTotalCache = std::min(nTotalCache, nMaxDbCache << 20); // total cache cannot be greater than nMaxDbcache
int64_t nBlockTreeDBCache = std::min(nTotalCache / 8, nMaxBlockDBCache << 20);
nTotalCache -= nBlockTreeDBCache;
int64_t nTxIndexCache = std::min(nTotalCache / 8, gArgs.GetBoolArg("-txindex", DEFAULT_TXINDEX) ? nMaxTxIndexCache << 20 : 0);
nTotalCache -= nTxIndexCache;
int64_t nCoinDBCache = std::min(nTotalCache / 2, (nTotalCache / 4) + (1 << 23)); // use 25%-50% of the remainder for disk cache
nCoinDBCache = std::min(nCoinDBCache, nMaxCoinsDBCache << 20); // cap total coins db cache
nTotalCache -= nCoinDBCache;
nCoinCacheUsage = nTotalCache; // the rest goes to in-memory cache
int64_t nMempoolSizeMax = gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000;
LogPrintf("Cache configuration:\n");
LogPrintf("* Using %.1fMiB for block index database\n", nBlockTreeDBCache * (1.0 / 1024 / 1024));
if (gArgs.GetBoolArg("-txindex", DEFAULT_TXINDEX)) {
LogPrintf("* Using %.1fMiB for transaction index database\n", nTxIndexCache * (1.0 / 1024 / 1024));
}
LogPrintf("* Using %.1fMiB for chain state database\n", nCoinDBCache * (1.0 / 1024 / 1024));
LogPrintf("* Using %.1fMiB for in-memory UTXO set (plus up to %.1fMiB of unused mempool space)\n", nCoinCacheUsage * (1.0 / 1024 / 1024), nMempoolSizeMax * (1.0 / 1024 / 1024));

Also, "-prune" is fixed:

  1. The GUI values in GB are translated to the node values in MiB correctly.
  2. The maximum of the "prune" QSpinBox is not limited by default value of 99 (GB).

Fix: #15106

@fanquake fanquake added the GUI label Jan 14, 2019

@DrahtBot

This comment has been minimized.

Copy link
Contributor

commented Jan 14, 2019

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #13676 (Explain that mempool memory is added to -dbcache by Sjors)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

@laanwj

This comment has been minimized.

Copy link
Member

commented Jan 14, 2019

Concept ACK it is good to be consistent with units.

@MarcoFalke

This comment has been minimized.

Copy link
Member

commented Jan 14, 2019

@hebasto hebasto changed the title Correct unit for "-dbcache" Correct units for "-dbcache" and "-prune" Jan 14, 2019

@hebasto

This comment has been minimized.

Copy link
Member Author

commented Jan 14, 2019

@MarcoFalke
Thank you for your review. Your comment has been addressed.

src/qt/intro.h Outdated
@@ -10,6 +10,7 @@
#include <QThread>

static const bool DEFAULT_CHOOSE_DATADIR = false;
static constexpr uint64_t GB_BYTES = 1000000000LL;

This comment has been minimized.

Copy link
@laanwj

laanwj Jan 15, 2019

Member

I'd propose put this in guiconstants.h instead, there is no reason optionsdialog.cpp should depend on intro.h

@hebasto hebasto force-pushed the hebasto:20190114-correct-info-units branch to 2c75758 Jan 15, 2019

@hebasto

This comment has been minimized.

Copy link
Member Author

commented Jan 15, 2019

@laanwj
Thank you for your review. Your comment has been addressed.

@laanwj

This comment has been minimized.

Copy link
Member

commented Jan 21, 2019

utACK 2c75758

@@ -52,4 +52,7 @@ static const int MAX_URI_LENGTH = 255;
#define QAPP_APP_NAME_TESTNET "Bitcoin-Qt-testnet"
#define QAPP_APP_NAME_REGTEST "Bitcoin-Qt-regtest"

/* One gigabyte (GB) in bytes */
static constexpr uint64_t GB_BYTES{1000000000};

This comment has been minimized.

Copy link
@laanwj

laanwj Jan 24, 2019

Member

Why is the LL suffix no longer needed here? Due to the new initialization style?

This comment has been minimized.

Copy link
@sipa

sipa Jan 24, 2019

Member

@laanwj I was wondering about the same thing, and this is what I conclude.

Since C++11, L, LL, UL, ULL should never be needed anymore, as the type of the constant is automatically chosen to be sufficiently large for the provided value, but signed. Before C++11, long long would not be automatically chosen.

It may be necessary to use a U suffix to indicate an unsigned constant, as a 64-bit value (which is larger or equal to 2^63) will by default be of type signed long (long), which would overflow.

This comment has been minimized.

Copy link
@hebasto

hebasto Jan 24, 2019

Author Member

Due to the new initialization style?

Yes. list initialization (since C++11):

Narrowing conversions

list-initialization limits the allowed implicit conversions by prohibiting ... conversion from integer ... to integer type that cannot represent all values of the original, except where source is a constant expression whose value can be stored exactly in the target type

This comment has been minimized.

Copy link
@laanwj
@jonasschnelli

This comment has been minimized.

Copy link
Member

commented Jan 29, 2019

Needs a commit squash... otherwise utACK

Correct units for "-dbcache" and "-prune"
All dbcache-related values in the code are measured in MiB (not in
megabytes, MB) or in bytes.
The GUI "-prune" values in GB are translated to the node values in MiB
correctly. The maximum of the "-prune" QSpinBox is not limited by the
default value of 99 (GB).
Also, this improves log readability.

@hebasto hebasto force-pushed the hebasto:20190114-correct-info-units branch from 2c75758 to 6f6514a Jan 30, 2019

@hebasto

This comment has been minimized.

Copy link
Member Author

commented Jan 30, 2019

Squashed.

@laanwj laanwj merged commit 6f6514a into bitcoin:master Jan 30, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

laanwj added a commit that referenced this pull request Jan 30, 2019

Merge #15163: Correct units for "-dbcache" and "-prune"
6f6514a Correct units for "-dbcache" and "-prune" (Hennadii Stepanov)

Pull request description:

  Actually, all `dbcache`-related values in the code are measured in MiB (not in megabytes, MB) or in bytes (e.g., `nTotalCache`).

  See: https://github.com/bitcoin/bitcoin/blob/master/src/txdb.h

  https://github.com/bitcoin/bitcoin/blob/ba8c8b22272ad40fe2de465d7e745532bab48d3b/src/init.cpp#L1405-L1424

  Also, "-prune" is fixed:
  1. The GUI values in GB are translated to the node values in MiB correctly.
  2. The maximum of the "prune" `QSpinBox` is not limited by default value of 99 (GB).

  Fix: #15106

Tree-SHA512: 151ec43b31b1074db8b345fedb1dcc10bde225899a5296bfc183f57e1553d13ac27db8db100226646769ad03c9fcab29d88763065a471757c6c41ac51108459d

@hebasto hebasto deleted the hebasto:20190114-correct-info-units branch Jan 30, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.