bench: add fluent API for untimed setup steps in nanobench #34208

pull l0rinc wants to merge 2 commits into bitcoin:master from l0rinc:l0rinc/untimed-setup-nanobench changing 6 files +101 −59
  1. l0rinc commented at 11:45 am on January 6, 2026: contributor

    Context

    As described in https://github.com/martinus/nanobench/issues/130, we have a few benchmarks where we have to reset the state between runs, otherwise the repetitions will do something else than the first iteration.

    Upstream

    I have opened a PR to nanobench to introduce an untimed setup phase, see: https://github.com/martinus/nanobench/pull/136 Unfortunately the author has health issues and couldn’t contribute to it in the past 2 years.

    Tests

    Tests were only added upstream, would be a bit awkward to wire in nanobench.h outside the benchmarking setup: https://github.com/martinus/nanobench/pull/136/changes/58350cfe5912d7e0533ccfcd759eea5d62eb55bb#diff-88160f647ce57661afe7d755fa70a5fa342a2b79d72d3511596878e69ed5cdc3

    Fix

    I have moved the changes here as well and applied it to a few simple benchmarks as demonstration. We can revert ones that are controversial and add other ones in follow-ups, this PR is mostly meant to add the setup feature.

    Benchmarks

    Most benchmarks show a modest “speedup”, others a “slowdown” - but it’s only the effect of the setup that’s not measured anymore - and a run phase which does the same operation in each epoch iteration (wallet benchmark changes were reverted since for simplicity):

  2. DrahtBot added the label Tests on Jan 6, 2026
  3. DrahtBot commented at 11:45 am on January 6, 2026: 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/34208.

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    ACK janb84
    Concept ACK sipa
    Stale ACK bensig

    If your review is incorrectly listed, please copy-paste <!–meta-tag:bot-skip–> into the comment that the bot should ignore.

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #32729 (test,refactor: extract script template helpers & widen sigop count coverage by l0rinc)
    • #32554 (bench: replace embedded raw block with configurable block generator by l0rinc)
    • #31868 ([IBD] specialize block serialization by l0rinc)
    • #31682 ([IBD] specialize CheckBlock’s input & coinbase checks by l0rinc)

    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.

  4. DrahtBot added the label CI failed on Jan 6, 2026
  5. DrahtBot commented at 1:02 pm on January 6, 2026: contributor

    🚧 At least one of the CI tasks failed. Task tidy: https://github.com/bitcoin/bitcoin/actions/runs/20747321605/job/59567659454 LLM reason (✨ experimental): clang-tidy failed the build due to a use-after-move error in wallet_loading.cpp, causing the CI to fail.

    Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

    • Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

    • A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.

    • An intermittent issue.

    Leave a comment here, if you need help tracking down a confusing failure.

  6. l0rinc force-pushed on Jan 6, 2026
  7. janb84 commented at 2:21 pm on January 6, 2026: contributor

    ACK e9e3ea1341caeab87aef8cb25dc0b58f7c3b27c8

    PR patches Vendored Code from Nanobench to include untimed setup phase.

    Personally I like the fluent style way we can now setup a benchmark, after the patch. The patch looks solid and is backwards compatible. A downside (i see) of patching vendored code is that it now contains “untested” code. (Yes it’s only benchmark code, so not on a critical path, I get it) I has tests upstream 👍

    Have thought about how to “fix” this, maybe include a “test” benchmark that shows the code works (with and without the setup) but that does not proves a lot imho. Anyhow not blocking.

  8. l0rinc commented at 2:26 pm on January 6, 2026: contributor

    vendored code is that it now contains “untested” code

    Thanks for the review, please see the tests in the upstream PR: https://github.com/martinus/nanobench/pull/136/changes/58350cfe5912d7e0533ccfcd759eea5d62eb55bb#diff-88160f647ce57661afe7d755fa70a5fa342a2b79d72d3511596878e69ed5cdc3

  9. DrahtBot removed the label CI failed on Jan 6, 2026
  10. bensig commented at 9:54 pm on January 8, 2026: contributor

    ACK e9e3ea1341caeab87aef8cb25dc0b58f7c3b27c8

    • Tested -
  11. DrahtBot added the label Needs rebase on Jan 14, 2026
  12. l0rinc force-pushed on Jan 15, 2026
  13. l0rinc commented at 12:50 pm on January 15, 2026: contributor
    Rebased after the benchmark priority cleanup. Ready for review again.
  14. DrahtBot removed the label Needs rebase on Jan 15, 2026
  15. DrahtBot added the label Needs rebase on Jan 21, 2026
  16. bench: add fluent API for untimed setup steps in `nanobench` 
    Running a few benchmarks (which will be migrated in the next commit to use the new setup method) several times to showcase the spread:

./build/bin/bench_bitcoin -filter='^(BnBExhaustion|AddrManAddThenGood|DeserializeBlockTest|DeserializeAndCheckBlockTest|CheckBlockTest|LoadExternalBlockFile|FindByte|WalletCreatePlain|WalletCreateEncrypted|WalletLoadingDescriptors)$'

|               ns/op |                op/s |    err% |     total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
|       26,400,542.00 |               37.88 |    0.4% |      0.29 | `AddrManAddThenGood`
|          189,075.00 |            5,288.91 |    0.4% |      0.01 | `BnBExhaustion`

|            ns/block |             block/s |    err% |     total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
|        1,237,000.00 |              808.41 |    2.4% |      0.01 | `DeserializeAndCheckBlockTest`
|          893,333.00 |            1,119.40 |    0.6% |      0.01 | `DeserializeBlockTest`

|               ns/op |                op/s |    err% |     total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
|               31.62 |       31,622,370.70 |    0.2% |      0.01 | `FindByte`
|        5,506,875.00 |              181.59 |    1.4% |      0.06 | `LoadExternalBlockFile`
|      593,480,333.00 |                1.68 |    0.4% |      6.53 | `WalletCreateEncrypted`
|      174,305,167.00 |                5.74 |    0.7% |      1.93 | `WalletCreatePlain`
|      160,833,875.00 |                6.22 |    0.2% |      0.80 | `WalletLoadingDescriptors`

|               ns/op |                op/s |    err% |     total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
|       26,005,125.00 |               38.45 |    1.3% |      0.29 | `AddrManAddThenGood`
|          181,909.67 |            5,497.23 |    0.1% |      0.01 | `BnBExhaustion`

|            ns/block |             block/s |    err% |     total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
|        1,223,000.00 |              817.66 |    2.8% |      0.01 | `DeserializeAndCheckBlockTest`
|          892,917.00 |            1,119.92 |    0.7% |      0.01 | `DeserializeBlockTest`

|               ns/op |                op/s |    err% |     total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
|               31.58 |       31,660,608.70 |    0.5% |      0.01 | `FindByte`
|        5,612,750.00 |              178.17 |    1.1% |      0.06 | `LoadExternalBlockFile`
|      594,012,250.00 |                1.68 |    0.2% |      6.53 | `WalletCreateEncrypted`
|      174,668,334.00 |                5.73 |    0.8% |      1.92 | `WalletCreatePlain`
|      158,494,375.00 |                6.31 |    0.3% |      0.79 | `WalletLoadingDescriptors`

Note that the `Default is 1ms, so we are mostly relying ...` change is simply an update from latest nanobench master.
    45e6558ba7
  17. l0rinc force-pushed on Jan 21, 2026
  18. DrahtBot removed the label Needs rebase on Jan 21, 2026
  19. DrahtBot added the label CI failed on Jan 22, 2026
  20. DrahtBot commented at 0:25 am on January 22, 2026: contributor

    🚧 At least one of the CI tasks failed. Task tidy: https://github.com/bitcoin/bitcoin/actions/runs/21228258011/job/61081107788 LLM reason (✨ experimental): CI failed due to a clang-tidy error (unused return value in coin_selection.cpp).

    Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

    • Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

    • A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.

    • An intermittent issue.

    Leave a comment here, if you need help tracking down a confusing failure.

  21. refactor: improve benchmark setup and execution for various tests 
    Note that `make_hard_case` already clears the UTXO pool in `coin_selection.cpp`.

./build/bin/bench_bitcoin -filter='^(BnBExhaustion|AddrManAddThenGood|DeserializeBlockTest|DeserializeAndCheckBlockTest|CheckBlockTest|LoadExternalBlockFile|FindByte|WalletCreatePlain|WalletCreateEncrypted|WalletLoadingDescriptors)$'

|               ns/op |                op/s |    err% |     total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
|       15,088,500.00 |               66.28 |    0.2% |      0.17 | `AddrManAddThenGood`
|          179,208.00 |            5,580.11 |    2.0% |      0.00 | `BnBExhaustion`

|            ns/block |             block/s |    err% |     total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
|          318,166.00 |            3,143.01 |    3.5% |      0.00 | `CheckBlockTest`
|          886,750.00 |            1,127.71 |    0.8% |      0.01 | `DeserializeBlockTest`

|               ns/op |                op/s |    err% |     total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
|               42.00 |       23,809,523.81 |    2.4% |      0.00 | `FindByte`
|        5,473,208.00 |              182.71 |    0.4% |      0.06 | `LoadExternalBlockFile`
|      584,168,041.00 |                1.71 |    0.3% |      6.43 | `WalletCreateEncrypted`
|      168,040,458.00 |                5.95 |    1.1% |      1.85 | `WalletCreatePlain`
|      155,446,625.00 |                6.43 |    0.7% |      0.78 | `WalletLoadingDescriptors`

|               ns/op |                op/s |    err% |     total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
|       14,894,917.00 |               67.14 |    0.3% |      0.16 | `AddrManAddThenGood`
|          177,667.00 |            5,628.51 |    1.3% |      0.00 | `BnBExhaustion`

|            ns/block |             block/s |    err% |     total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
|          313,791.00 |            3,186.83 |    3.8% |      0.00 | `CheckBlockTest`
|          888,208.00 |            1,125.86 |    0.7% |      0.01 | `DeserializeBlockTest`

|               ns/op |                op/s |    err% |     total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
|               41.00 |       24,390,243.90 |    2.4% |      0.00 | `FindByte`
|        5,445,208.00 |              183.65 |    1.0% |      0.06 | `LoadExternalBlockFile`
|      581,800,500.00 |                1.72 |    0.4% |      6.40 | `WalletCreateEncrypted`
|      166,035,583.00 |                6.02 |    0.5% |      1.82 | `WalletCreatePlain`
|      153,574,792.00 |                6.51 |    0.1% |      0.77 | `WalletLoadingDescriptors`
    1f4c9b6f09
  22. l0rinc force-pushed on Jan 22, 2026
  23. DrahtBot removed the label CI failed on Jan 22, 2026
  24. janb84 commented at 12:40 pm on January 23, 2026: contributor

    re ACK 1f4c9b6f093522cdbdd55e9f99b239106e6d6daa

    Removing the setup time from the actual benchmark timing will result in a more representative benchmark, still think this is an improvement over benchmarking with setup steps included.

  25. sipa commented at 9:18 pm on January 27, 2026: member
    Concept ACK. This would help with a number of benchmarks I’ve written before.


l0rinc DrahtBot janb84 bensig sipa

Labels
Tests

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-02-02 06:13 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me