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

pull sedited wants to merge 1 commits into bitcoin:master from sedited:txdownloadman_ibd_check changing 1 files +1 −1
  1. sedited commented at 9:06 am on December 12, 2025: contributor

    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.

  2. DrahtBot added the label P2P on Dec 12, 2025
  3. DrahtBot commented at 9:06 am on December 12, 2025: contributor

    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 <!–meta-tag:bot-skip–> into the comment that the bot should ignore.

    Conflicts

    No conflicts as of last run.

  4. maflcko commented at 10:17 am on December 12, 2025: member

    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

  5. sedited commented at 8:21 pm on December 15, 2025: contributor

    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.

  6. maflcko commented at 12:32 pm on December 16, 2025: member

    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?

  7. sedited commented at 12:52 pm on December 16, 2025: contributor

    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.

  8. maflcko commented at 1:31 pm on December 16, 2025: member
    Oh, I see. I guess if the behavior should be enforced at compile-time, the blocks could be std::moved into the event
  9. DrahtBot added the label Needs rebase on Dec 16, 2025
  10. sedited force-pushed on Dec 16, 2025
  11. sedited commented at 3:26 pm on December 16, 2025: contributor

    Rebased 2233420fcff7c9c3d7d5727880a8ae1e6173eb89 -> 42dc1725b978329e0808db5e2c3a64c038c35de3 (txdownloadman_ibd_check_0 -> txdownloadman_ibd_check_1, compare)

  12. DrahtBot removed the label Needs rebase on Dec 16, 2025
  13. in src/net_processing.cpp:2057 in 42dc1725b9 outdated
    1966@@ -1967,7 +1967,7 @@ void PeerManagerImpl::BlockConnected(
    1967 
    1968     // The following task can be skipped since we don't maintain a mempool for
    1969     // the historical chainstate.
    


    mzumsande commented at 8:46 pm on December 23, 2025:
    comment could be adjusted for why we skip it during IBD.
  14. mzumsande commented at 9:16 pm on December 23, 2025: contributor

    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.

  15. sedited commented at 9:32 am on December 24, 2025: contributor

    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?

  16. fjahr commented at 9:28 pm on December 25, 2025: contributor
    Concept ACK
  17. mzumsande commented at 6:51 pm on December 26, 2025: contributor

    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?

  18. maflcko commented at 10:46 am on January 12, 2026: member

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

  19. l0rinc commented at 2:44 pm on January 16, 2026: contributor

    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:

     0COMMITS="13891a8a685d255cb13dd5018e3d5ccc18b07c34 42dc1725b978329e0808db5e2c3a64c038c35de3"; \
     1STOP=931139; DBCACHE=4500; PRUNE=550; \
     2CC=gcc; CXX=g++; \
     3BASE_DIR="/mnt/my_storage"; DATA_DIR="$BASE_DIR/ShallowBitcoinData"; LOG_DIR="$BASE_DIR/logs"; \
     4(echo ""; for c in $COMMITS; do git fetch -q origin $c && git log -1 --pretty='%h %s' $c || exit 1; done) && \
     5(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 "") &&\
     6hyperfine \
     7  --sort command \
     8  --runs 2 \
     9  --export-json "$BASE_DIR/ibd-$(sed -E 's/(\w{8})\w+ ?/\1-/g;s/-$//'<<<"$COMMITS")-$STOP-$DBCACHE-$CC.json" \
    10  --parameter-list COMMIT ${COMMITS// /,} \
    11  --prepare "killall -9 bitcoind 2>/dev/null; rm -rf $DATA_DIR/*; git clean -fxd; git reset --hard {COMMIT} && \
    12    cmake -B build -G Ninja -DCMAKE_BUILD_TYPE=Release && ninja -C build bitcoind -j2 && \
    13    ./build/bin/bitcoind -datadir=$DATA_DIR -stopatheight=1 -prune=$PRUNE -printtoconsole=0; sleep 20" \
    14  --conclude "killall bitcoind || true; sleep 5; grep -q 'height=0' $DATA_DIR/debug.log && grep -q 'Disabling script verification at block [#1](/bitcoin-bitcoin/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})"; \
    15              cp $DATA_DIR/debug.log $LOG_DIR/debug-{COMMIT}-$(date +%s).log" \
    16  "COMPILER=$CC ./build/bin/bitcoind -datadir=$DATA_DIR -stopatheight=$STOP -dbcache=$DBCACHE -blocksonly -prune=$PRUNE -printtoconsole=0"
    17
    1813891a8a68 Merge bitcoin/bitcoin#34050: fuzz: exercise `ComputeMerkleRoot` without `mutated` parameter
    1942dc1725b9 net processing: Check if we are in ibd before processing block for txdownloadman
    20
    212026-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
    22
    23Benchmark 1: COMPILER=gcc ./build/bin/bitcoind -datadir=/mnt/my_storage/ShallowBitcoinData -stopatheight=931139 -dbcache=4500 -blocksonly -prune=550 -printtoconsole=0 (COMMIT = 13891a8a685d255cb13dd5018e3d5ccc18b07c34)
    24  Time (mean ± σ):     32486.491 s ± 552.685 s    [User: 68289.029 s, System: 4354.310 s]
    25  Range (min  max):   32095.684 s  32877.298 s    2 runs
    26 
    27Benchmark 2: COMPILER=gcc ./build/bin/bitcoind -datadir=/mnt/my_storage/ShallowBitcoinData -stopatheight=931139 -dbcache=4500 -blocksonly -prune=550 -printtoconsole=0 (COMMIT = 42dc1725b978329e0808db5e2c3a64c038c3
    28  Time (mean ± σ):     32647.522 s ± 1124.203 s    [User: 67219.308 s, System: 4265.439 s]
    29  Range (min  max):   31852.591 s  33442.454 s    2 runs
    30 
    31Relative speed comparison
    32        1.00          COMPILER=gcc ./build/bin/bitcoind -datadir=/mnt/my_storage/ShallowBitcoinData -stopatheight=931139 -dbcache=4500 -blocksonly -prune=550 -printtoconsole=0 (COMMIT = 13891a8a685d255cb13dd5018e3d5ccc18b07c34)
    33        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:

     0for 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} && \
     1      cmake -B build -G Ninja -DCMAKE_BUILD_TYPE=Release -DENABLE_IPC=OFF && ninja -C build bitcoind -j1 && \
     2      ./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](/bitcoin-bitcoin/1/)' $DATA_DIR/debug.log && grep -q 'height=$STOP' $DATA_DIR/debug.log; \
     3                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
     4
     513891a8a68 Merge bitcoin/bitcoin#34050: fuzz: exercise `ComputeMerkleRoot` without `mutated` parameter
     642dc1725b9 net processing: Check if we are in ibd before processing block for txdownloadman
     7
     82026-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
     9
    10Benchmark 1: COMPILER=gcc ./build/bin/bitcoind -datadir=/mnt/my_storage/BitcoinData -stopatheight=931139 -dbcache=450 -reindex-chainstate -blocksonly -connect=0 -printtoconsole=0 (COMMIT = 13891a8a685d255cb13dd5018e3d5ccc18b07c34)
    11  Time (abs ):        21626.602 s               [User: 49157.755 s, System: 3039.249 s]
    12
    13Benchmark 2: COMPILER=gcc ./build/bin/bitcoind -datadir=/mnt/my_storage/BitcoinData -stopatheight=931139 -dbcache=450 -reindex-chainstate -blocksonly -connect=0 -printtoconsole=0 (COMMIT = 42dc1725b978329e0808db5e2c3a64c038c35de3)
    14  Time (abs ):        21755.087 s               [User: 47668.251 s, System: 2952.652 s]
    15
    16Relative speed comparison
    17        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)
    18        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)
    19
    2013891a8a68 Merge bitcoin/bitcoin#34050: fuzz: exercise `ComputeMerkleRoot` without `mutated` parameter
    2142dc1725b9 net processing: Check if we are in ibd before processing block for txdownloadman
    22
    232026-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
    24
    25Benchmark 1: COMPILER=gcc ./build/bin/bitcoind -datadir=/mnt/my_storage/BitcoinData -stopatheight=931139 -dbcache=4500 -reindex-chainstate -blocksonly -connect=0 -printtoconsole=0 (COMMIT = 13891a8a685d255cb13dd5018e3d5ccc18b07c34)
    26  Time (abs ):        17263.550 s               [User: 33148.005 s, System: 945.859 s]
    27
    28Benchmark 2: COMPILER=gcc ./build/bin/bitcoind -datadir=/mnt/my_storage/BitcoinData -stopatheight=931139 -dbcache=4500 -reindex-chainstate -blocksonly -connect=0 -printtoconsole=0 (COMMIT = 42dc1725b978329e0808db5e2c3a64c038c35de3)
    29  Time (abs ):        17480.331 s               [User: 31675.951 s, System: 739.137 s]
    30
    31Relative speed comparison
    32        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)
    33        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)
    
  20. maflcko commented at 2:52 pm on January 16, 2026: member

    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?

  21. maflcko commented at 2:54 pm on January 16, 2026: member
    I am still thinking if the std::move patch (https://github.com/bitcoin/bitcoin/pull/34054#issuecomment-3660534822) makes sense to include here.
  22. l0rinc commented at 2:59 pm on January 16, 2026: contributor

    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.

    0sudo cat ../BitcoinData/debug.log | egrep 'height=0|height=690000'                                 
    12026-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)
    22026-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.

  23. sedited referenced this in commit 1c2f164d34 on Jan 29, 2026
  24. net processing: Check if we are in ibd before processing block for txdownloadman
    This avoids wasting work on calculating bloom filters that aren't
    consumed during ibd and continuously re-calculated as now blocks get
    validated.
    62383b7949
  25. sedited force-pushed on Jan 29, 2026
  26. sedited commented at 11:04 am on January 29, 2026: contributor

    Rebased 42dc1725b978329e0808db5e2c3a64c038c35de3 -> 62383b79498a6e6293f22790674deecfa3516d19 (txdownloadman_ibd_check_1 -> txdownloadman_ibd_check_2, compare)

    • Rebased to get changes from #34253 .

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-01-30 00:13 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me