Open
Conversation
Add Bench::setup(...).run(...) for untimed per-epoch setup. The existing run() now delegates to runImpl() with an empty setup lambda. Tests verify setup is excluded from timing (50ms sleep → ~0 measured) and runs once before each epoch's iterations (S-RRR-S-RRR pattern).
This was referenced Jan 3, 2026
|
For symmetry you should also have The only problem I foresee is that with fluent API it becomes a little unintuitive with Bench::setup(...).teardown(...).run(...) |
Author
|
We can do that in a follow-up, but a setup should already fix most cases since we usually need the benchmarking conditions to be met - and tearing it down can either be done in the next setup run or when existing the benchmark. |
achow101
added a commit
to bitcoin/bitcoin
that referenced
this pull request
Apr 6, 2026
…bench 8825051 refactor: improve benchmark setup and execution for various tests (Lőrinc) 83b8528 bench: add fluent API for untimed setup steps in `nanobench` (Lőrinc) Pull request description: ### Context As described in martinus/nanobench#130, we have a few benchmarks where we have to reset the state between runs; otherwise, the repetitions will do something different than the first iteration. ### Upstream I have opened a PR to `nanobench` to introduce an untimed setup phase, see: martinus/nanobench#136 ### Tests Tests were only added upstream. It would be a bit awkward to wire them into `nanobench.h` outside the benchmarking setup: martinus/nanobench@58350cf#diff-88160f647ce57661afe7d755fa70a5fa342a2b79d72d3511596878e69ed5cdc3 ### Fix I have moved the changes here as well and applied them to a few simple benchmarks as a demonstration. We can revert the ones that are controversial and add others 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 that does the same operation in each epoch iteration (wallet benchmark changes were reverted for simplicity): <img width="1496" height="882" alt="image" src="https://github.com/user-attachments/assets/34c14565-f3df-41e5-9a86-95b2ca21703a" /> ACKs for top commit: achow101: ACK 8825051 janb84: re ACK 8825051 sedited: ACK 8825051 Tree-SHA512: b3e385abcfca013a21b3785b0b837c2b61e302d71a098dadcd8d2f0cb42f6bbf4a222299771443f095962d1b24e696d5684f2b8efdb6f63f2f939699961cdf0d
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add
Bench::setup(...).run(...)for untimed per-epoch setup.The existing
run()now delegates torunImpl()with an empty setup lambda.Tests verify setup is excluded from timing (50ms sleep → ~0 measured) and runs once before each epoch's iterations (S-RRR-S-RRR pattern).
Fixes: #130
Alternative to: #135