net processing: Add ibd check before processing block for txdownloadman #34054

sedited · 2025-12-12T09:06:28Z

Calculating the rolling bloom filters for the txorphanage takes some CPU time from the scheduler thread. This can be observed for example in this flamegraph, where handling the filter takes about 2.6% of total time (and most of the scheduler thread's time).

During ibd the entries in the tx download bloom filter are just continuously rolled over and aren't consumed, since no mempool entries are created by incoming transactions from peers during ibd. The mempool does accept transactions via RPC, or the wallet at the time, however these don't interact with the orphanage and the txdownloadman, because adding anything to those is guarded by IsInitialBlockDownload() checks as well.

We're usually latching ibd to false a few blocks before catching up to the tip, so this should also not significantly degrade the performance of the filter once fully caught up.

DrahtBot · 2025-12-12T09:06:37Z

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

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/34054.

Reviews

See the guideline for information on the review process.

Type	Reviewers
Concept ACK	mzumsande, fjahr

If your review is incorrectly listed, please copy-paste  into the comment that the bot should ignore.

Conflicts

No conflicts as of last run.

maflcko · 2025-12-12T10:17:21Z

During ibd the entries in the tx download bloom filter are just continuously rolled over, and aren't consumed, since no mempool is maintained at the time.

I think this could be clarified a bit, why this is safe. My understanding is that a mempool does exist and is maintained (albeit it will normally be empty). The mempool will also happily accept transactions via RPC, or the wallet. However, this is fine, because they don't interact with the orphanage and the txdownloadman, because adding anything to those is guarded by IsInitialBlockDownload() checks as well?

Ref:

https://github.com/bitcoin/bitcoin/blob/8da8ff30a57f0fe7f627d121c5e40466f1ecb0b2/src/net_processing.cpp#L4258-L4268

and

https://github.com/bitcoin/bitcoin/blob/8da8ff30a57f0fe7f627d121c5e40466f1ecb0b2/src/net_processing.cpp#L3987-L3990

sedited · 2025-12-15T20:21:26Z

Re #34054 (comment)

I think this could be clarified a bit, why this is safe.

Thanks, that was not precise. Slightly reworded your explanation and added it to the pull request description. I was curious if the ibd check might cause a performance regression, since we're probably triggering fewer de-allocs on the scheduler thread. I tried to get some results in benchcoin. The results were kind of mixed, but there is clearly no more work being done through the txdownloadman during ibd.

maflcko · 2025-12-16T12:32:13Z

fewer de-allocs on the scheduler thread.

I think this patch does not change whether the event is run on the scheduler thread, or not, but rather if the event runs faster in the scheduler thread with the early return. So I think this should not move destructor calls between threads. Or am I missing something?

sedited · 2025-12-16T12:52:18Z

Re #34054 (comment)

but rather if the event runs faster in the scheduler thread with the early return.

Yes, and I was curious if this might change where the final decrement for the block's shared pointer happens.

maflcko · 2025-12-16T13:31:05Z

Oh, I see. I guess if the behavior should be enforced at compile-time, the blocks could be std::moved into the event

…downloadman

sedited · 2025-12-16T15:26:36Z

Rebased 2233420 -> 42dc172 (txdownloadman_ibd_check_0 -> txdownloadman_ibd_check_1, compare)

Fixed conflict with refactor: Improve assumeutxo state representation #30214

mzumsande

Concept ACK

In addition to RPC/wallet mentioned above, the mempool could also contain transactions from disk - downloaded at a time when we weren't in IBD. This is very common for nodes that aren't online 24/7 , so that they will go back into IBD at each subsequent start. But those shouldn't really be a problem either I think.

Also, this introduces a cs_main lock in the scheduler thread, which could cause some lock contention with msghand - not sure how important this it, but it might go well together with #32885.

mzumsande · 2025-12-23T20:46:30Z

src/net_processing.cpp

    }

    // The following task can be skipped since we don't maintain a mempool for
    // the historical chainstate.


comment could be adjusted for why we skip it during IBD.

sedited · 2025-12-24T09:32:25Z

Also, this introduces a cs_main lock in the scheduler thread, which could cause some lock contention with msghand - not sure how important this it, but it might go well together with #32885.

Yes, the original PR explicitly worked around this by introducing a variable to track the ibd state: https://github.com/bitcoin/bitcoin/pull/32730/files#diff-b1e19192258d83199d8adaa5ac31f067af98f63554bfdd679bd8e8073815e69dL1911-L1912 . A prior approach of that PR passes the ibd state directly in the BlockConnected callback. I think blocking here might be fine (which is why it opened this), but it's not great either. What do you think of drafting this for now, and we try to make progress with #32885, or do you prefer one of the other approaches?

fjahr · 2025-12-25T21:28:56Z

Concept ACK

mzumsande · 2025-12-26T18:51:43Z

What do you think of drafting this for now, and we try to make progress with #32885, or do you prefer one of the other approaches?

I think I like the approach from #32885 because it addresses the issue at its root (so that many IBD checks would benefit, not just this one). As to whether this should be drafted I'm unsure - it's probably an improvement even with the added locking due to the saved work, but given the mixed results from benchcoin maybe we'd need more testing?

maflcko · 2026-01-12T10:46:21Z

given the mixed results from benchcoin maybe we'd need more testing?

The latest runs shows a speedup in both scenarios, but the earlier run showed a slowdown in mainnet-default-uninstrumented, but the only difference was a rebase, so this seems to indicate that the benchcoin run itself is not stable?

(Commit 8da8ff30a57f0fe7f627d121c5e40466f1ecb0b2 was deleted by GitHub, so I am just going after the comment #34054 (comment))

l0rinc · 2026-01-16T14:44:05Z

The latest runs shows a speedup in both scenarios, but the earlier run showed a slowdown in mainnet-default-uninstrumented, but the only difference was a rebase, so this seems to indicate that the benchcoin run itself is not stable?

We're not really using benchcoin anymore - at least I haven't seen anyone take decisions based on it lately. It was an experiment we tried, I don't think it's viable anymore - as you also noticed.
I'm using the script below to check these and they're a lot more reliable and stable on the infrastructure I'm running them on.
I ran a few pruned IBDs over real nodes, these were stable and show no change in IBD speed:

COMMITS="13891a8a685d255cb13dd5018e3d5ccc18b07c34 42dc1725b978329e0808db5e2c3a64c038c35de3"; \
STOP=931139; DBCACHE=4500; PRUNE=550; \
CC=gcc; CXX=g++; \
BASE_DIR="/mnt/my_storage"; DATA_DIR="$BASE_DIR/ShallowBitcoinData"; LOG_DIR="$BASE_DIR/logs"; \
(echo ""; for c in $COMMITS; do git fetch -q origin $c && git log -1 --pretty='%h %s' $c || exit 1; done) && \
(echo "" && echo "$(date -I) | pruned IBD | ${STOP} blocks | dbcache ${DBCACHE} | pruning ${PRUNE} | $(hostname) | $(uname -m) | $(lscpu | grep 'Model name' | head -1 | cut -d: -f2 | xargs) | $(nproc) cores | $(free -h | awk '/^Mem:/{print $2}') RAM | $(df -T $BASE_DIR | awk 'NR==2{print $2}') | $(lsblk -no ROTA $(df --output=source $BASE_DIR | tail -1) | grep -q 0 && echo SSD || echo HDD)"; echo "") &&\
hyperfine \
  --sort command \
  --runs 2 \
  --export-json "$BASE_DIR/ibd-$(sed -E 's/(\w{8})\w+ ?/\1-/g;s/-$//'<<<"$COMMITS")-$STOP-$DBCACHE-$CC.json" \
  --parameter-list COMMIT ${COMMITS// /,} \
  --prepare "killall -9 bitcoind 2>/dev/null; rm -rf $DATA_DIR/*; git clean -fxd; git reset --hard {COMMIT} && \
    cmake -B build -G Ninja -DCMAKE_BUILD_TYPE=Release && ninja -C build bitcoind -j2 && \
    ./build/bin/bitcoind -datadir=$DATA_DIR -stopatheight=1 -prune=$PRUNE -printtoconsole=0; sleep 20" \
  --conclude "killall bitcoind || true; sleep 5; grep -q 'height=0' $DATA_DIR/debug.log && grep -q 'Disabling script verification at block #1' $DATA_DIR/debug.log && grep -q 'height=$STOP' $DATA_DIR/debug.log && grep 'Bitcoin Core version' $DATA_DIR/debug.log | grep -q "$(printf %.12s {COMMIT})"; \
              cp $DATA_DIR/debug.log $LOG_DIR/debug-{COMMIT}-$(date +%s).log" \
  "COMPILER=$CC ./build/bin/bitcoind -datadir=$DATA_DIR -stopatheight=$STOP -dbcache=$DBCACHE -blocksonly -prune=$PRUNE -printtoconsole=0"

13891a8a68 Merge bitcoin/bitcoin#34050: fuzz: exercise `ComputeMerkleRoot` without `mutated` parameter
42dc1725b9 net processing: Check if we are in ibd before processing block for txdownloadman

2026-01-14 | pruned IBD | 931139 blocks | dbcache 4500 | pruning 550 | i9-ssd | x86_64 | Intel(R) Core(TM) i9-9900K CPU @ 3.60GHz | 16 cores | 62Gi RAM | xfs | SSD

Benchmark 1: COMPILER=gcc ./build/bin/bitcoind -datadir=/mnt/my_storage/ShallowBitcoinData -stopatheight=931139 -dbcache=4500 -blocksonly -prune=550 -printtoconsole=0 (COMMIT = 13891a8a685d255cb13dd5018e3d5ccc18b07c34)
  Time (mean ± σ):     32486.491 s ± 552.685 s    [User: 68289.029 s, System: 4354.310 s]
  Range (min … max):   32095.684 s … 32877.298 s    2 runs
 
Benchmark 2: COMPILER=gcc ./build/bin/bitcoind -datadir=/mnt/my_storage/ShallowBitcoinData -stopatheight=931139 -dbcache=4500 -blocksonly -prune=550 -printtoconsole=0 (COMMIT = 42dc1725b978329e0808db5e2c3a64c038c3
  Time (mean ± σ):     32647.522 s ± 1124.203 s    [User: 67219.308 s, System: 4265.439 s]
  Range (min … max):   31852.591 s … 33442.454 s    2 runs
 
Relative speed comparison
        1.00          COMPILER=gcc ./build/bin/bitcoind -datadir=/mnt/my_storage/ShallowBitcoinData -stopatheight=931139 -dbcache=4500 -blocksonly -prune=550 -printtoconsole=0 (COMMIT = 13891a8a685d255cb13dd5018e3d5ccc18b07c34)
        1.00 ±  0.04  COMPILER=gcc ./build/bin/bitcoind -datadir=/mnt/my_storage/ShallowBitcoinData -stopatheight=931139 -dbcache=4500 -blocksonly -prune=550 -printtoconsole=0 (COMMIT = 42dc1725b978329e0808db5e2c3a64c038c35de3)

Similarly for -reindex-chainstate - which was probably unaffected anyway since now download happened, but it's a good sanity-check anyway:

for DBCACHE in 450 4500; do   COMMITS="13891a8a685d255cb13dd5018e3d5ccc18b07c34 42dc1725b978329e0808db5e2c3a64c038c35de3";   STOP=931139; CC=gcc; CXX=g++;   BASE_DIR="/mnt/my_storage"; DATA_DIR="$BASE_DIR/BitcoinData"; LOG_DIR="$BASE_DIR/logs";   (echo ""; for c in $COMMITS; do git fetch -q origin $c && git log -1 --pretty='%h %s' $c || exit 1; done) &&   (echo "" && echo "$(date -I) | reindex-chainstate | ${STOP} blocks | dbcache ${DBCACHE} | $(hostname) | $(uname -m) | $(lscpu | grep 'Model name' | head -1 | cut -d: -f2 | xargs) | $(nproc) cores | $(free -h | awk '/^Mem:/{print $2}') RAM | SSD"; echo "") &&  hyperfine     --sort command     --runs 1     --export-json "$BASE_DIR/rdx-$(sed -E 's/(\w{8})\w+ ?/\1-/g;s/-$//'<<<"$COMMITS")-$STOP-$DBCACHE-$CC.json"     --parameter-list COMMIT ${COMMITS// /,}     --prepare "killall -9 bitcoind 2>/dev/null; rm -f $DATA_DIR/debug.log; git clean -fxd; git reset --hard {COMMIT} && \
      cmake -B build -G Ninja -DCMAKE_BUILD_TYPE=Release -DENABLE_IPC=OFF && ninja -C build bitcoind -j1 && \
      ./build/bin/bitcoind -datadir=$DATA_DIR -stopatheight=$STOP -dbcache=1000 -printtoconsole=0; sleep 20; rm -f $DATA_DIR/debug.log"     --conclude "killall bitcoind || true; sleep 5; grep -q 'height=0' $DATA_DIR/debug.log && grep -q 'Disabling script verification at block #1' $DATA_DIR/debug.log && grep -q 'height=$STOP' $DATA_DIR/debug.log; \
                cp $DATA_DIR/debug.log $LOG_DIR/debug-{COMMIT}-$(date +%s).log"     "COMPILER=$CC ./build/bin/bitcoind -datadir=$DATA_DIR -stopatheight=$STOP -dbcache=$DBCACHE -reindex-chainstate -blocksonly -connect=0 -printtoconsole=0"; done

13891a8a68 Merge bitcoin/bitcoin#34050: fuzz: exercise `ComputeMerkleRoot` without `mutated` parameter
42dc1725b9 net processing: Check if we are in ibd before processing block for txdownloadman

2026-01-13 | reindex-chainstate | 931139 blocks | dbcache 450 | i9-ssd | x86_64 | Intel(R) Core(TM) i9-9900K CPU @ 3.60GHz | 16 cores | 62Gi RAM | SSD

Benchmark 1: COMPILER=gcc ./build/bin/bitcoind -datadir=/mnt/my_storage/BitcoinData -stopatheight=931139 -dbcache=450 -reindex-chainstate -blocksonly -connect=0 -printtoconsole=0 (COMMIT = 13891a8a685d255cb13dd5018e3d5ccc18b07c34)
  Time (abs ≡):        21626.602 s               [User: 49157.755 s, System: 3039.249 s]

Benchmark 2: COMPILER=gcc ./build/bin/bitcoind -datadir=/mnt/my_storage/BitcoinData -stopatheight=931139 -dbcache=450 -reindex-chainstate -blocksonly -connect=0 -printtoconsole=0 (COMMIT = 42dc1725b978329e0808db5e2c3a64c038c35de3)
  Time (abs ≡):        21755.087 s               [User: 47668.251 s, System: 2952.652 s]

Relative speed comparison
        1.00          COMPILER=gcc ./build/bin/bitcoind -datadir=/mnt/my_storage/BitcoinData -stopatheight=931139 -dbcache=450 -reindex-chainstate -blocksonly -connect=0 -printtoconsole=0 (COMMIT = 13891a8a685d255cb13dd5018e3d5ccc18b07c34)
        1.01          COMPILER=gcc ./build/bin/bitcoind -datadir=/mnt/my_storage/BitcoinData -stopatheight=931139 -dbcache=450 -reindex-chainstate -blocksonly -connect=0 -printtoconsole=0 (COMMIT = 42dc1725b978329e0808db5e2c3a64c038c35de3)

13891a8a68 Merge bitcoin/bitcoin#34050: fuzz: exercise `ComputeMerkleRoot` without `mutated` parameter
42dc1725b9 net processing: Check if we are in ibd before processing block for txdownloadman

2026-01-14 | reindex-chainstate | 931139 blocks | dbcache 4500 | i9-ssd | x86_64 | Intel(R) Core(TM) i9-9900K CPU @ 3.60GHz | 16 cores | 62Gi RAM | SSD

Benchmark 1: COMPILER=gcc ./build/bin/bitcoind -datadir=/mnt/my_storage/BitcoinData -stopatheight=931139 -dbcache=4500 -reindex-chainstate -blocksonly -connect=0 -printtoconsole=0 (COMMIT = 13891a8a685d255cb13dd5018e3d5ccc18b07c34)
  Time (abs ≡):        17263.550 s               [User: 33148.005 s, System: 945.859 s]

Benchmark 2: COMPILER=gcc ./build/bin/bitcoind -datadir=/mnt/my_storage/BitcoinData -stopatheight=931139 -dbcache=4500 -reindex-chainstate -blocksonly -connect=0 -printtoconsole=0 (COMMIT = 42dc1725b978329e0808db5e2c3a64c038c35de3)
  Time (abs ≡):        17480.331 s               [User: 31675.951 s, System: 739.137 s]

Relative speed comparison
        1.00          COMPILER=gcc ./build/bin/bitcoind -datadir=/mnt/my_storage/BitcoinData -stopatheight=931139 -dbcache=4500 -reindex-chainstate -blocksonly -connect=0 -printtoconsole=0 (COMMIT = 13891a8a685d255cb13dd5018e3d5ccc18b07c34)
        1.01          COMPILER=gcc ./build/bin/bitcoind -datadir=/mnt/my_storage/BitcoinData -stopatheight=931139 -dbcache=4500 -reindex-chainstate -blocksonly -connect=0 -printtoconsole=0 (COMMIT = 42dc1725b978329e0808db5e2c3a64c038c35de3)

maflcko · 2026-01-16T14:52:16Z

Nice. Thanks for running those. I guess it is expected that there is no difference, unless one is running on a single-core-single-thread system, because the scheduler thread runs in the background.

I guess the flamegraphs from benchcoin are still useful and indicate that the schedulder now eats 50% less background CPU?

maflcko · 2026-01-16T14:54:32Z

I am still thinking if the std::move patch (#34054 (comment)) makes sense to include here.

l0rinc · 2026-01-16T14:59:52Z

unless one is running on a single-core-single-thread system

I don't think we support those. The lousiest machine I have is an rpi4 with 2 GB memory (even this has nproc = 4) and we can't even compile on that machine without OOMing - unless we do some tricks with ccache).
I'm only at 50% of the blocks after almost 9 days.

progress=0.503054 after 8 days, 18 hours, 55 minutes, 22 seconds

sudo cat ../BitcoinData/debug.log | egrep 'height=0|height=690000'                                 
2026-01-07T19:49:56Z UpdateTip: new best=000000000019d6689c085ae165831e934ff763ae46a2a6c172b3f1b60a8ce26f height=0 version=0x00000001 log2_work=32.000022 tx=1 date='2009-01-03T18:15:05Z' progress=0.000000 cache=0.3MiB(0txo)
2026-01-16T14:45:18Z UpdateTip: new best=00000000000000000002a23d6df20eecec15b21d32c75833cce28f113de888b7 height=690000 version=0x20000004 log2_work=92.968172 tx=653953301 date='2021-07-07T07:43:07Z' progress=0.503054 cache=80.5MiB(612179txo)

The PR might still make sense, I haven't investigated it in detail yet.

DrahtBot added the P2P label Dec 12, 2025

maflcko mentioned this pull request Dec 12, 2025

p2p: avoid traversing blocks (twice) during IBD #32730

Closed

DrahtBot mentioned this pull request Dec 16, 2025

refactor: Improve assumeutxo state representation #30214

Merged

net processing: Check if we are in ibd before processing block for tx…

42dc172

…downloadman

DrahtBot added the Needs rebase label Dec 16, 2025

sedited force-pushed the txdownloadman_ibd_check branch from 8da8ff3 to 42dc172 Compare December 16, 2025 15:26

kevkevinpal mentioned this pull request Dec 16, 2025

refactor: Improve assumeutxo state representation kevkevinpal/bitcoin#231

Closed

DrahtBot removed the Needs rebase label Dec 16, 2025

mzumsande reviewed Dec 23, 2025

View reviewed changes

l0rinc mentioned this pull request Jan 11, 2026

validation: cache tip recency for lock-free IsInitialBlockDownload() #34253

Open

l0rinc mentioned this pull request Jan 14, 2026

Coins Cache Cleanup checklist #34280

Open

24 tasks

net processing: Add ibd check before processing block for txdownloadman #34054

Are you sure you want to change the base?

net processing: Add ibd check before processing block for txdownloadman #34054

Conversation

sedited commented Dec 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DrahtBot commented Dec 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage & Benchmarks

Reviews

Conflicts

Uh oh!

maflcko commented Dec 12, 2025

Uh oh!

sedited commented Dec 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maflcko commented Dec 16, 2025

Uh oh!

sedited commented Dec 16, 2025

Uh oh!

maflcko commented Dec 16, 2025

Uh oh!

sedited commented Dec 16, 2025

Uh oh!

mzumsande left a comment

Choose a reason for hiding this comment

Uh oh!

mzumsande Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

sedited commented Dec 24, 2025

Uh oh!

fjahr commented Dec 25, 2025

Uh oh!

mzumsande commented Dec 26, 2025

Uh oh!

maflcko commented Jan 12, 2026

Uh oh!

l0rinc commented Jan 16, 2026

Uh oh!

maflcko commented Jan 16, 2026

Uh oh!

maflcko commented Jan 16, 2026

Uh oh!

l0rinc commented Jan 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

sedited commented Dec 12, 2025 •

edited

Loading

DrahtBot commented Dec 12, 2025 •

edited

Loading

sedited commented Dec 15, 2025 •

edited

Loading

l0rinc commented Jan 16, 2026 •

edited

Loading