Don't empty dbcache on prune flushes: >30% faster IBD#28280
Don't empty dbcache on prune flushes: >30% faster IBD#28280achow101 merged 14 commits intobitcoin:masterfrom
Conversation
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. Code CoverageFor detailed information about the code coverage, see the test coverage report. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. ConflictsReviewers, this pull request conflicts with the following ones:
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. |
andrewtoth
force-pushed
the
sync-dirty
branch
3 times, most recently
from
f5b20ca to
97b6bb5
Compare
andrewtoth
force-pushed
the
sync-dirty
branch
5 times, most recently
from
2f78edb to
7bdbf31
Compare
andrewtoth
force-pushed
the
sync-dirty
branch
3 times, most recently
from
d35cea3 to
b8ccdac
Compare
|
Concept ACK! I'll try to repro the bench results in the next week or so. |
|
@andrewtoth Not sure if you have seen #20827 which is a different approach but has a similar effect. |
|
@0xB10C indeed I have seen that one. I believe they complement each other. #20827 reduces IBD time for larger |
At the time I had a spare and otherwise idle machine set up I could use. I don't have the bash script at hand but IIRC I had a merge base (MB) and a pull-request (PR) binary I'd run with different I've uploaded the jupyter nodebooks here https://gist.github.com/0xB10C/89c2903290cfb1840792d41dcd079646. The first was used for a partial IBD from 500k-600k and the second one with a full IBD. Both times syncing from a local node on the same host. The notebooks expect the debug log files with the name |
|
Bench running now, should have some results in the next day or so. $ ( bitcoinperf --no-teardown bench-pr --num-blocks 30_000 --run-id no-flush-on-prune \
--bitcoind-args='-dbcache=4000 -prune=550' --run-count 3 28280 && \
pushover 'Bench for andrewtoth SUCCEEDED!' ) || \
pushover 'Bench for andrewtoth failed' |
|
Local IBD of 30_000 blocks with |
|
reACK 589db87 |
|
ACK 589db87 |
|
🎉 Thanks everyone! |
|
Ported to the CMake-based build system in hebasto#318. |
16 participants
Since #17487 we no longer need to clear the coins cache when syncing to disk. A warm coins cache significantly speeds up block connection, and only needs to be fully flushed when nearing the
dbcachelimit.For frequent pruning flushes there's no need to empty the cache and kill connect block speed. However, simply using
Syncin place ofFlushactually slows down a pruned full IBD with a highdbcachevalue. This is because as the cache grows, sync takes longer since every coin in the cache is scanned to check if it's dirty. For frequent prune flushes and a large cache this constant scanning starts to really slow IBD down, and just emptying the cache on every prune becomes faster.To fix this, we can add two pointers to each cache entry and construct a doubly linked list of dirty entries. We can then only iterate through all dirty entries on each
Sync, and simply clear the pointers after.With this approach a full IBD with
dbcache=16384andprune=550was 32% faster than master. For defaultdbcache=450speedup was ~9%. All benchmarks were run withstopatheight=800000.While the 2 pointers add memory to each cache entry, it did not slow down IBD. For non-pruned IBD results were similar for this branch and master. When I performed the initial IBD, the full UTXO set could be held in memory when using the max
dbcachevalue. For non-pruned IBD with maxdbcacheto tip ended up using 12% more memory, but it was also 2.7% faster somehow. For smallerdbcachevalues thedbcachelimit is respected so does not consume more memory, and the potentially more frequent flushes were not significant enough to cause any slowdown.For reviewers, the commits in order do the following:
First 4 commits encapsulate all accesses to
flagson cache entries, and then the 5th makesflagsprivate.Commits
refactor: add CoinsCachePair aliastocoins: call ClearFlags in CCoinsCacheEntry destructorcreate the linked list head nodes and cache entry self references and pass them intoAddFlags.Commit
coins: track flagged cache entries in linked listactually adds the entries into a linked list when they are flagged DIRTY or FRESH and removes them from the linked list when they are destroyed or the flags are cleared manually. However, the linked list is not yet used anywhere.Commit
test: add cache entry linked list testsadds unit tests for the linked list.Commit
coins: pass linked list of flagged entries to BatchWriteuses the linked list to iterate through DIRTY entries instead of using the entire coins cache.Commit
validation: don't erase coins cache on prune flushesusesSyncinstead ofFlushfor pruning flushes, so the cache is no longer cleared.Inspired by this comment.
Fixes #11315.