Skip to content

benchmarks: restore benchmarks#182

Merged
apoelstra merged 3 commits intorust-bitcoin:masterfrom
apoelstra:2024-03--restore-benches
Jul 6, 2024
Merged

benchmarks: restore benchmarks#182
apoelstra merged 3 commits intorust-bitcoin:masterfrom
apoelstra:2024-03--restore-benches

Conversation

@apoelstra
Copy link
Copy Markdown
Member

In #128 we dropped the benchmarks. They can be restored with only minor tweaks, so do so.

@apoelstra
Copy link
Copy Markdown
Member Author

CI failure is just because nightly rustc has introduced a new Freeze trait which appears in the check-API check. There is a commit to fix this in #180 which I won't copy into here to avoid rebase trouble.

Copy link
Copy Markdown
Member

@tcharding tcharding left a comment

Choose a reason for hiding this comment

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

ACK 82a577d

@apoelstra
Copy link
Copy Markdown
Member Author

cc @clarkmoody this is a quick/easy PR. Can you do a quick review?

Alternately, would you be okay with my merging things here with only ACKs from @tcharding?

@tcharding
Copy link
Copy Markdown
Member

Needs api files updating @apoelstra

@tcharding
Copy link
Copy Markdown
Member

Also I've emailed Clark to see what is the status here and on #180.

Copy link
Copy Markdown
Member

@clarkmoody clarkmoody left a comment

Choose a reason for hiding this comment

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

ACK 82a577d

apoelstra added 3 commits July 6, 2024 13:10
Found with `cargo +nightly clippy --all-targets`.
In rust-bitcoin#128 we dropped the benchmarks. They can be restore with only minor
tweaks, so do so.
Mainly to confirm that these are really expensive (on my system,
mulitple milliseconds for the descriptor checksum) and that we should
precompute these values rather than suggesting users compute them at
run-time whenever they want to do correction (which would result in a
simpler API).
@apoelstra apoelstra force-pushed the 2024-03--restore-benches branch from 82a577d to e0b95f1 Compare July 6, 2024 13:13
@apoelstra
Copy link
Copy Markdown
Member Author

Rebased, added a one-line warning fix from my error-correction-2 branch, and also added a benchmark for parameter generation.

Copy link
Copy Markdown
Member

@clarkmoody clarkmoody left a comment

Choose a reason for hiding this comment

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

ACK e0b95f1

@apoelstra apoelstra merged commit 86f7dc0 into rust-bitcoin:master Jul 6, 2024
@apoelstra apoelstra deleted the 2024-03--restore-benches branch July 6, 2024 14:26
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.

3 participants