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

hebasto · 2019-01-14T12:23:01Z

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:

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

Fix: #15106

DrahtBot · 2019-01-14T13:47:44Z

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 · 2019-01-14T14:41:31Z

Concept ACK it is good to be consistent with units.

maflcko · 2019-01-14T16:12:53Z

Should do the same for prune? https://github.com/bitcoin/bitcoin/search?utf8=%E2%9C%93&q=550MB&type=

hebasto · 2019-01-14T23:08:10Z

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

laanwj · 2019-01-15T14:09:22Z

src/qt/intro.h

@@ -10,6 +10,7 @@
 #include <QThread>

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


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

hebasto · 2019-01-15T17:06:06Z

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

laanwj · 2019-01-21T16:43:08Z

utACK 2c75758

laanwj · 2019-01-24T15:49:09Z

src/qt/guiconstants.h

@@ -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};


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

@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.

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

Thanks @sipa and @hebasto

jonasschnelli · 2019-01-29T20:30:23Z

Needs a commit squash... otherwise utACK

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 · 2019-01-30T05:27:48Z

Squashed.

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

Summary: 6f6514a08090b37b5e8c086015ee4881813ef867 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 --- Backport of Core [[bitcoin/bitcoin#15163 | PR15163]] Test Plan: ninja all check check-functional Reviewers: #bitcoin_abc, jasonbcox Reviewed By: #bitcoin_abc, jasonbcox Differential Revision: https://reviews.bitcoinabc.org/D7896

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: bitcoin#15106 Tree-SHA512: 151ec43b31b1074db8b345fedb1dcc10bde225899a5296bfc183f57e1553d13ac27db8db100226646769ad03c9fcab29d88763065a471757c6c41ac51108459d

fanquake added the GUI label Jan 14, 2019

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

laanwj reviewed Jan 15, 2019

View reviewed changes

hebasto force-pushed the 20190114-correct-info-units branch from 2050357 to 2c75758 Compare January 15, 2019 16:57

laanwj reviewed Jan 24, 2019

View reviewed changes

hebasto force-pushed the 20190114-correct-info-units branch from 2c75758 to 6f6514a Compare January 30, 2019 05:26

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

hebasto deleted the 20190114-correct-info-units branch January 30, 2019 19:24

Sjors mentioned this pull request Jan 31, 2019

Explain that mempool memory is added to -dbcache #13676

Merged

bitcoin locked as resolved and limited conversation to collaborators Dec 16, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

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

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

Uh oh!

hebasto commented Jan 14, 2019 •

edited

Loading

Uh oh!

DrahtBot commented Jan 14, 2019 •

edited

Loading

Uh oh!

laanwj commented Jan 14, 2019

Uh oh!

maflcko commented Jan 14, 2019

Uh oh!

hebasto commented Jan 14, 2019

Uh oh!

laanwj Jan 15, 2019

Uh oh!

hebasto commented Jan 15, 2019

Uh oh!

laanwj commented Jan 21, 2019

Uh oh!

laanwj Jan 24, 2019

Uh oh!

sipa Jan 24, 2019 •

edited

Loading

Uh oh!

hebasto Jan 24, 2019

Uh oh!

laanwj Jan 24, 2019

Uh oh!

jonasschnelli commented Jan 29, 2019

Uh oh!

hebasto commented Jan 30, 2019

Uh oh!

Uh oh!

	// 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));

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

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

Uh oh!

Conversation

hebasto commented Jan 14, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DrahtBot commented Jan 14, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Conflicts

Uh oh!

laanwj commented Jan 14, 2019

Uh oh!

maflcko commented Jan 14, 2019

Uh oh!

hebasto commented Jan 14, 2019

Uh oh!

laanwj Jan 15, 2019

Choose a reason for hiding this comment

Uh oh!

hebasto commented Jan 15, 2019

Uh oh!

laanwj commented Jan 21, 2019

Uh oh!

laanwj Jan 24, 2019

Choose a reason for hiding this comment

Uh oh!

sipa Jan 24, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hebasto Jan 24, 2019

Choose a reason for hiding this comment

Uh oh!

laanwj Jan 24, 2019

Choose a reason for hiding this comment

Uh oh!

jonasschnelli commented Jan 29, 2019

Uh oh!

hebasto commented Jan 30, 2019

Uh oh!

Uh oh!

hebasto commented Jan 14, 2019 •

edited

Loading

DrahtBot commented Jan 14, 2019 •

edited

Loading

sipa Jan 24, 2019 •

edited

Loading