Bump `-dbcache` default to 300MiB

laanwj commented at 2:00 pm on June 27, 2016: member

Bump -dbcache default value to 300MiB.

Also cap the allocations for the leveldb-specific caches to 32MiB, the current default for 100MiB. This avoids that the extra cache memory goes to the much less effective leveldb cache instead of our application-level cache.

Before:

`-dbcache`	`-txindex`	block index db (MiB)	chain state db (MiB)	in-memory UTXO set (MiB)
100 default	0	2.0	32.5	65.5
100	1	12.5	29.9	57.6
300	0	2.0	82.5	215.5
300	1	37.5	73.6	188.9

After:

`-dbcache`	`-txindex`	block index db (MiB)	chain state db (MiB)	in-memory UTXO set (MiB)
100	0	2.0	8.0	90.0
100	1	12.5	29.9	57.6
300 default	0	2.0	8.0	290.0
300	1	8.0	8.0	284.0

Number in bold is the size allocated to the CCoinsCache in the default case.

See #8245, as well as 2016-06-23 meeting log for discussion.

Note: after this, we also need something like #8138 - currently the dbcache determines how deep undo data is rolled back at start, so otherwise this will result in the start-up verification becoming even slower.

laanwj added the label UTXO Db and Indexes on Jun 27, 2016

laanwj commented at 2:00 pm on June 27, 2016: member

Specific numbers are of course open for discussion.

laanwj added this to the milestone 0.13.0 on Jun 27, 2016

MarcoFalke commented at 6:39 pm on June 27, 2016: member

Would this require mention in the release notes?

Imagine someone updating on weak hardware, which does not support the new memory requirement.

jonasschnelli commented at 7:21 pm on June 27, 2016: contributor

This is a good idea and we should def. do this!

Long term I would wish a flexible solution. If you have to catch up a couple of days/week (or during IBD), bitcoind/qt should use a catchup-cache-setting (can be a different, second -dbcacheibd [or similar] settings). Or we could think about adding “profiles” (set-changes of some CPU/MEM related settings).

jonasschnelli commented at 7:22 pm on June 27, 2016: contributor

utACK https://github.com/bitcoin/bitcoin/pull/8273/commits/fdf085fcaccd012a54ead0de4c01cf35f541f257 ACK for 0.13. Would need prominent releasenotes part.

in src/txdb.h: in fdf085fcac outdated

31+//! Max memory allocated to block tree DB specific cache (if no -txindex)
32+static const int64_t nMaxBlockDBCache = 2 << 20;
33+//! Max memory allocated to block tree DB specific cache (with -txindex)
34+static const int64_t nMaxBlockDBAndTxIndexCache = 32 << 20;
35+//! Max memory allocated to coin DB specific cache
36+static const int64_t nMaxCoinsDBCache = 32 << 20;

dcousens commented at 4:37 am on June 28, 2016:

nit: without knowing the shorthand by heart, it isn’t exactly clear what << 20 represents here. It could have some implied context compared to the fact its just shorthand for MiB (assuming I’ve understood it correctly!)

laanwj commented at 7:50 am on June 28, 2016:

yes, I don’t want to mention numbers in the comments (as everyone forgets to update those as they’re not checked by the compiler) but specifying that it’s MiB is fine with me

dcousens commented at 4:38 am on June 28, 2016: contributor

concept ACK

laanwj commented at 7:51 am on June 28, 2016: member

Would this require mention in the release notes?

Yes.

Imagine someone updating on weak hardware, which does not support the new memory requirement.

Well at least now that the mempool is capped, that ought to free up some memory for other uses. But yes it could be the final straw, sure…

Long term I would wish a flexible solution. If you have to catch up a couple of days/week (or during IBD), bitcoind/qt should use a catchup-cache-setting (can be a different, second -dbcacheibd [or similar] settings).

Right - during initial sync (and catch-up) the mempool is virtually guaranteed to be empty. This fact could be used to temporarily bump dbcache. But it’s too late for that for 0.13.

MarcoFalke commented at 8:39 am on June 28, 2016: member

Concept ACK for .13

in src/txdb.h: in fdf085fcac outdated

21@@ -22,11 +22,17 @@ class CCoinsViewDBCursor;
22 class uint256;
23 
24 //! -dbcache default (MiB)
25-static const int64_t nDefaultDbCache = 100;
26+static const int64_t nDefaultDbCache = 300;
27 //! max. -dbcache in (MiB)

MarcoFalke commented at 8:41 am on June 28, 2016:

Nit: braces should be “(in MiB)” or remove “in”

in src/txdb.h: in fdf085fcac outdated

30 static const int64_t nMinDbCache = 4;
31+//! Max memory allocated to block tree DB specific cache (if no -txindex)
32+static const int64_t nMaxBlockDBCache = 2 << 20;
33+//! Max memory allocated to block tree DB specific cache (with -txindex)
34+static const int64_t nMaxBlockDBAndTxIndexCache = 32 << 20;
35+//! Max memory allocated to coin DB specific cache

MarcoFalke commented at 8:42 am on June 28, 2016:

Nit: Mention “(in bytes)” or “(bytes)”

laanwj commented at 10:01 am on June 28, 2016:

bah, it’s kind of messy that some of these are in bytes, and some in MiB We probably should switch them all to bytes, or all to MiB.

laanwj commented at 1:18 pm on June 28, 2016:

All the constants are in MiB now, and that’s documented as such

gmaxwell commented at 11:35 am on June 28, 2016: contributor

I am (slowly) trying various leveldb cache sizes to see how big it really needs to be.

laanwj force-pushed on Jun 28, 2016

gmaxwell commented at 7:09 pm on June 28, 2016: contributor

On a Xeon-D 1541 with 64GB ram, 7200 rpm disk, with no txindex, and all signature checks disabled, I found that:

Capping GetOptions ncachesize to 1MB (holding all else constant) resulted in a 1% performance loss and capping at 4MB to a 0.1% loss (31072.23 to reindex instead of 31021.76). (2MB was very close to the 1MB numbers). On this basis I recommend clamping at leveldb 4MB or 8MB (just in case future growth or other workloads makes it matter more), and handing the rest of the memory to the coins cache.

laanwj commented at 7:23 am on June 29, 2016: member

Capping GetOptions ncachesize to 1MB (holding all else constant) resulted in a 1% performance loss and capping at 4MB to a 0.1% loss (31072.23 to reindex instead of 31021.76). (2MB was very close to the 1MB numbers). On this basis I recommend clamping at leveldb 4MB or 8MB (just in case future growth or other workloads makes it matter more), and handing the rest of the memory to the coins cache.

Thanks for testing! I’ll change the maximums to 8 MiB.

laanwj referenced this in commit db7573f0ae on Jun 29, 2016

gmaxwell commented at 5:34 pm on June 29, 2016: contributor

Benchmarked this pull req on the above system with reindex (and no signature checking at all), went from 31021.76 seconds to 21303.22.

It’s still a lot slower than the ‘infinite’ dbcache size results; but a huge improvement over the old results.

ACK.

roques commented at 6:22 pm on June 29, 2016: contributor

@gmaxwell I recently enabled txindex and reindexed. Reindexing was IO-bound by LevelDB compacting. When setting -dbcache=16384 I observed that LevelDB’s Level-0 files became much larger (~350MB) than with the default settings. This resulted in top LevelDB compactions of 4@0 + 0@1 files => 1458007320 bytes, resulting in several hundred new level-1 files of which the great majority can simply be moved into higher levels instead of requiring to be compacted into higher levels. This saved me quite a few compactions and thus sped-up the whole reindex.

Unfortunately, I’m unfamiliar with the exact memory management of LevelDB and what memory is used to build these large Level-0 files. However, if this patch clamps that memory of LevelDB to 8MB it will not be able to create such large Level-0 files and thus will have to do quite a few more lower-level compactions. — @maxwell, it would be nice if you could time an ‘infinite dbcache’ reindex with txindex=1 with this patch and see if the patch hurts its performance or if my guess is wrong.

gmaxwell commented at 6:26 pm on June 29, 2016: contributor

@roques I started that test before commenting above. Will update when it’s done.

gmaxwell commented at 0:52 am on June 30, 2016: contributor

With dbcache set to 8192, reindex with this patch and no signature checks took 8071.65 seconds, and without 8071.65 seconds. Test with txindex isn’t done yet (it was later in my testqueue).

laanwj commented at 8:03 am on June 30, 2016: member

Could set static const int64_t nMaxBlockDBAndTxIndexCache = 8; back to 32, that would be safer. It’s possible that it makes more of a difference there then for the UTXO cache, as the access pattern is different, and I wasn’t really planning to make any changes for -txindex here anyway.

gmaxwell commented at 8:57 am on June 30, 2016: contributor

With txindex, no signature checks, and 8GB dbcache, before this PR 10131.43 and after 11682.22. So it makes a meaningful difference. I’m starting a test with the txindex case at 32 and seeing how that goes.

laanwj commented at 9:03 am on June 30, 2016: member

Added mention in release notes.

petertodd commented at 7:40 pm on June 30, 2016: contributor

Concept ACK

gmaxwell commented at 7:54 pm on June 30, 2016: contributor

10721.93 with txindex, infinite dbcache, and the 32MB clamp.

laanwj referenced this in commit 1009626d30 on Jul 1, 2016

laanwj commented at 7:50 am on July 1, 2016: member

Ok, thanks for measuring, so that still suffers. Removing the ceiling for txindex cache completely (well, bump it up to some unlikely number,1 GiB) in this PR. The exact performance compromise for txindex can be figured out later, this PR just aims to improve the common case.

jonasschnelli commented at 6:27 pm on July 1, 2016: contributor

Positing some slightly off topic results here:

Settings:

Current master da50997a3ee7d3b73180c71cd454580af49c2244
--disable-debug
-dbcache=8000
SSD

Results:

~3h04min IBD from random peers debug.log
~2h45min -reindex debug.log
~2h10min -reindex-chainstate debug.log

jonasschnelli commented at 7:10 pm on July 2, 2016: contributor

More data:

~5h22min default DB cache -reindex with master (da50997) debug.log
~4h06min default DB cache -reindex this PR (f2bdbb7) debug.log

jonasschnelli commented at 1:35 pm on July 4, 2016: contributor

Tested ACK 1009626d308ff70e3c0457559b87aacd510d0c42.

Bump `-dbcache` default to 300MiB

Also cap the allocation for the leveldb-specific cache for the UTXO set
to 8MiB.
This avoids that the extra cache memory goes to the much less effective
leveldb cache instead of our application-level cache.

32cab91278

doc: Mention dbcache increase in release notes efd1d8339a

laanwj force-pushed on Jul 6, 2016

laanwj commented at 5:46 am on July 6, 2016: member

Squashed and updated the commit messages for the new values.

laanwj merged this on Jul 6, 2016

laanwj closed this on Jul 6, 2016

laanwj referenced this in commit 396f9d6296 on Jul 6, 2016

codablock referenced this in commit 0259880424 on Sep 19, 2017

codablock referenced this in commit 54afcecb73 on Sep 27, 2017

codablock referenced this in commit da3f7ab3e5 on Oct 12, 2017

codablock referenced this in commit e3b7ed449f on Oct 19, 2017

UdjinM6 referenced this in commit c4926cfaa0 on Nov 8, 2017

AmirAbrams referenced this in commit d21c9f1858 on Aug 3, 2018

AmirAbrams referenced this in commit 38c087bef1 on Aug 4, 2018

furszy referenced this in commit 0e264e2607 on Jun 28, 2021

MarcoFalke locked this on Sep 8, 2021

Bump -dbcache default to 300MiB #8273

Bump `-dbcache` default to 300MiB #8273