sync: improve CCoinsViewCache ReallocateCache #28945

martinus · 2023-11-27T06:43:15Z

TL;DR: this change improves sync time on my PC by ~6%.

While syncing when the CCoinsViewCache gets full it is flushed to disk and then memory is freed. Depending on available cache size, this happens several times. This PR removes the unnecessary reallocations for temporary CCoinsViewCache and reserves the cacheCoins's bucket array to its previous size.

I benchmarked the change on an AMD Ryzen 9 7950X with this command:

hyperfine \
--show-output --parameter-list commit b5a271334ca81a6adcb1c608d85c83621a9eae47,ceeb71816512642e92a110b2cf5d2549d090fe78 \
--setup 'git checkout {commit} && make -j$(nproc) src/bitcoind' \
--prepare 'sync; sudo /sbin/sysctl vm.drop_caches=3' \
-M 3 './src/bitcoind -dbcache=500 -reindex-chainstate -printtoconsole=0 -stopatheight=500000'

Where b5a2713 is mergebase, and ceeb718 this PR.

Which runs ./src/bitcoind -dbcache=500 -reindex-chainstate -printtoconsole=0 -stopatheight=500000 3 times and compares both commits, giving this result:

Benchmark 1: ./src/bitcoind -dbcache=500 -reindex-chainstate -printtoconsole=0 -stopatheight=500000 (commit = b5a271334ca81a6adcb1c608d85c83621a9eae47)
  Time (mean ± σ):     2815.648 s ± 70.720 s    [User: 2639.959 s, System: 640.051 s]
  Range (min … max):   2736.616 s … 2872.964 s    3 runs

Benchmark 2: ./src/bitcoind -dbcache=500 -reindex-chainstate -printtoconsole=0 -stopatheight=500000 (commit = ceeb71816512642e92a110b2cf5d2549d090fe78)
  Time (mean ± σ):     2661.520 s ± 23.205 s    [User: 2473.010 s, System: 624.040 s]
  Range (min … max):   2635.816 s … 2680.926 s    3 runs

Summary
  ./src/bitcoind -dbcache=500 -reindex-chainstate -printtoconsole=0 -stopatheight=500000 (commit = ceeb71816512642e92a110b2cf5d2549d090fe78) ran
    1.06 ± 0.03 times faster than ./src/bitcoind -dbcache=500 -reindex-chainstate -printtoconsole=0 -stopatheight=500000 (commit = b5a271334ca81a6adcb1c608d85c83621a9eae47)

The short lived CCoinsViewCache's are destructed right after the Flush(), therefore it is not necessary to call ReallocateCache.

DrahtBot · 2023-11-27T06:43:19Z

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

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.
A summary of reviews will appear here.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#29236 (log: Nuke error(...) by maflcko)
#28280 (Don't empty dbcache on prune flushes: >30% faster IBD by andrewtoth)

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.

helpau · 2023-12-09T06:42:46Z

Hi, is this benchmark for HDD or SSD? Did you tried to run benchmark with huge dbcache and high height?

martinus · 2023-12-09T08:56:32Z

I only measured it on an SSD, and only with relatively low dbcache size so flushing is triggered more often which I think would give more accurate results. But more benchmarks are definitely appreciated :)

andrewtoth · 2023-12-16T22:03:37Z

src/coins.cpp

+
+    // After a flush all the cacheCoins's memory has now been freed. It is likely that the map gets full again so flush is necessary, and it will
+    // happen around the same memory usage. So we can already reserve the bucket array to the previous size, reducing the number of rehashes needed.
+    cacheCoins.reserve(current_bucket_count);


The purpose of ReallocateCache originally was to balance the snapshot and background chainstate caches (see #18637). However, in 5e4ac5a this function was removed from ResizeCoinsCaches and moved to Flush. This created an implicit assumption that calling Flush would clear the memory in the cache. This change violates that assumption, since the memory will no longer be clear and therefore ResizeCoinsCaches will likely no longer respect the dbcache limit during assumeutxo syncing.

Also see the docs for this function https://github.com/bitcoin/bitcoin/blob/master/src/coins.h#L336-L341. The purpose of this is to clear the memory of the cache.

Perhaps we could get the bucket count in Flush before calling ReallocateCache and reserve it in Flush, then re-add ReallocateCache to ResizeCoinsCaches?

edit: I see that reserve only allocates memory for the hashtable, and not the actual nodes stored in the unordered_map. I'm not sure how significant that extra memory is.

andrewtoth · 2023-12-16T22:43:09Z

src/coins.cpp

@@ -250,14 +250,16 @@ bool CCoinsViewCache::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlockIn
    return true;
 }

-bool CCoinsViewCache::Flush() {
+bool CCoinsViewCache::Flush(bool reallocate_cache) {


Should this parameter be added to the fuzz targets or unit tests?

andrewtoth · 2023-12-17T14:03:09Z

Ran same hyperfine benchmark with base, 985ddd9, and head of branch. Got similar results 🚀.

Summary
  ./src/bitcoind -dbcache=500 -reindex-chainstate -printtoconsole=0 -stopatheight=500000 (commit = ceeb71816512642e92a110b2cf5d2549d090fe78) ran
    1.06 ± 0.00 times faster than ./src/bitcoind -dbcache=500 -reindex-chainstate -printtoconsole=0 -stopatheight=500000 (commit = 985ddd954199cb7686ac75d12492d25b16d235c8)
    1.07 ± 0.00 times faster than ./src/bitcoind -dbcache=500 -reindex-chainstate -printtoconsole=0 -stopatheight=500000 (commit = b5a271334ca81a6adcb1c608d85c83621a9eae47)

It's unclear to me though why 5e4ac5a was necessary at all, and if reverting that change would be a cleaner fix for this instead?
From the commit message:

This is necessary in preparation for using the PoolAllocator for
CCoinsMap, which does not actually free any memory on clear().

Why is it necessary to free memory on clear()? Is the unfreed memory not just recycled for new coins as they are added? The only purpose of ReallocateCache seemed to be to clear all memory when rebalancing caches during assumeutxo syncing, and is unrelated to Flush.

DrahtBot · 2024-03-12T11:30:04Z

🐙 This pull request conflicts with the target branch and needs rebase.

fjahr

@martinus Are you still working on this? If yes, it would be great if you could rebase and address @andrewtoth 's comment. If not, I can try to take this over and get it the attention it deserves.

fjahr · 2024-04-27T19:39:41Z

src/coins.h

@@ -307,7 +307,7 @@ class CCoinsViewCache : public CCoinsViewBacked
     * to be forgotten.
     * If false is returned, the state of this cache (and its backing view) will be undefined.
     */
-    bool Flush();
+    bool Flush(bool reallocate_cache = true);


Should probably add something about this to the docstring.

martinus · 2024-04-27T20:17:31Z

@fjahr I'm not working on this any more, feel free to take over!

martinus added 3 commits November 27, 2023 06:51

coins: Add parameter to specify if ReallocateCache is necessary

83d10e9

validation: don't reallocate cache for short lived CCoinsViewCache

985ddd9

The short lived CCoinsViewCache's are destructed right after the Flush(), therefore it is not necessary to call ReallocateCache.

coins: reserve bucket count

ceeb718

DrahtBot mentioned this pull request Dec 1, 2023

Don't empty dbcache on prune flushes: >30% faster IBD #28280

Merged

andrewtoth reviewed Dec 16, 2023

View reviewed changes

DrahtBot mentioned this pull request Jan 11, 2024

log: Nuke error(...) #29236

Merged

DrahtBot added the CI failed label Jan 16, 2024

DrahtBot added the Needs rebase label Mar 12, 2024

achow101 requested a review from fjahr April 9, 2024 14:19

fjahr reviewed Apr 27, 2024

View reviewed changes

fanquake closed this Apr 28, 2024

andrewtoth mentioned this pull request Jul 1, 2024

optimization: Speed up Base58 encoding/decoding by 400%/200% via preliminary byte packing #29473

Closed

fjahr mentioned this pull request Jul 1, 2024

sync: improve CCoinsViewCache ReallocateCache - 2nd try #30370

Closed

This was referenced Mar 12, 2025

[IBD] Tracking PR for speeding up Initial Block Download #32043

Draft

ReallocateCache bitcoin-dev-tools/benchcoin#85

Open

bitcoin locked and limited conversation to collaborators Apr 28, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

sync: improve CCoinsViewCache ReallocateCache #28945

sync: improve CCoinsViewCache ReallocateCache #28945

Uh oh!

martinus commented Nov 27, 2023 •

edited

Loading

Uh oh!

DrahtBot commented Nov 27, 2023 •

edited

Loading

Uh oh!

helpau commented Dec 9, 2023

Uh oh!

martinus commented Dec 9, 2023

Uh oh!

andrewtoth Dec 16, 2023 •

edited

Loading

Uh oh!

andrewtoth Dec 16, 2023

Uh oh!

andrewtoth commented Dec 17, 2023

Uh oh!

DrahtBot commented Mar 12, 2024

Uh oh!

fjahr left a comment

Uh oh!

fjahr Apr 27, 2024

Uh oh!

martinus commented Apr 27, 2024

Uh oh!

Uh oh!

sync: improve CCoinsViewCache ReallocateCache #28945

sync: improve CCoinsViewCache ReallocateCache #28945

Uh oh!

Conversation

martinus commented Nov 27, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DrahtBot commented Nov 27, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage

Reviews

Conflicts

Uh oh!

helpau commented Dec 9, 2023

Uh oh!

martinus commented Dec 9, 2023

Uh oh!

andrewtoth Dec 16, 2023 • 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!

andrewtoth Dec 16, 2023

Choose a reason for hiding this comment

Uh oh!

andrewtoth commented Dec 17, 2023

Uh oh!

DrahtBot commented Mar 12, 2024

Uh oh!

fjahr left a comment

Choose a reason for hiding this comment

Uh oh!

fjahr Apr 27, 2024

Choose a reason for hiding this comment

Uh oh!

martinus commented Apr 27, 2024

Uh oh!

Uh oh!

martinus commented Nov 27, 2023 •

edited

Loading

DrahtBot commented Nov 27, 2023 •

edited

Loading

andrewtoth Dec 16, 2023 •

edited

Loading