Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rewrite (non-compiler) benchmarks #745

Merged
merged 2 commits into from
Nov 12, 2024

Conversation

apoelstra
Copy link
Member

This PR introduces a much larger and more comprehensive set of benchmarks for parsing miniscripts. It now covers:

  • Balanced and deep segwit scripts
  • Balanced and deep Taproot scripts inside a single leaf (which are allowed to be much bigger than segwit ones)
  • All of the above using thresh(2,A,B) as a combinator (which forces a Vec allocation and a bunch of different codepaths vs any of the other combinators)
  • Balanced and deep Taproot trees with pk() as leaves
  • A wide range of sizes for all of the above.

This doesn't touch the compiler benchmarks, mainly because it seemed like a bunch of work and I don't have any immediate intentions of messing with the compiler. But also because I'm not sure how to approach these -- with the parser I can just do giant trees of pk()s but with the compiler I probably want to use all of pk/hash/timelock etc.

By having everything in one file it reduces clutter in the codebase,
will make it easier to unify benchmark-generation code, and (IMO most
importantly) lets you write `cargo bench benchmarks` and then it'll only
run the benchmarks instead of outputting a gazillion "ignored" lines
telling you that it's not benchmarking random shit that aren't
benchmarks.

Don't re-format stuff so it'll be obvious in the diff that this is a
pure code move. Also we are going to rewrite the benchmarks in the next
commit anyway.
This commit doesn't redo the compiler benchmarks because they are not
essential to the changes I am planning to make.
Copy link
Member

@sanket1729 sanket1729 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK b0d9500.

Sorry for the delay

@apoelstra apoelstra merged commit ae8f450 into rust-bitcoin:master Nov 12, 2024
30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants