Skip to content

Refactors to avoid broken benchmarks#788

Merged
vapourismo merged 7 commits intomainfrom
kurtis/durable-storage/bench-feature-refactor
Feb 11, 2026
Merged

Refactors to avoid broken benchmarks#788
vapourismo merged 7 commits intomainfrom
kurtis/durable-storage/bench-feature-refactor

Conversation

@kurtisc
Copy link
Contributor

@kurtisc kurtisc commented Feb 5, 2026 •
edited
Loading

No ticket

What

This PR aims to ensure that the benchmarks are built and reduce the surface area of conditional compilation. There is a roughly linear relationship in opinionated-ness in each commit.

Why

A recent commit broke one of the benchmarks. This isn't the first time it has happened: they're not built as part of normal development or the CI.

How

  • Make bench part of the dev-dependencies
  • Fix the broken benchmark
  • Make some items pub but in non-pub modules, then re-exports them
  • Removes unused Merkle layer methods
  • Removes Get operations from the benchmark. We never get nodes - we just hash them!
  • Moves test-only code into the test modules

We still need a "bench" feature, but only in two places. One of those is the TestableTmpDir which I'm planning to move in #781I've moved in #803 , so would be able to re-export in the same way in whichever (if ever) gets merged last.

Divan might make it possible to remove the feature altogether.

Manually Testing

make all

^-- no cargo bench!

Tasks for the Author

  • Link all Linear issues related to this MR using magic words (e.g. part of, relates to, closes).
  • Eliminate dead code and other spurious artefacts introduced in your changes.
  • Document new public functions, methods and types.
  • Make sure the documentation for updated functions, methods, and types is correct.
  • Add tests for bugs that have been fixed.
  • Explain changes to regression test captures when applicable.
  • Write commit messages in agreement with our guidelines.
  • Self-review your changes to ensure they are high-quality.
  • Complete all of the above before assigning this MR to reviewers.

@codecov
Copy link

codecov bot commented Feb 5, 2026 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.07%. Comparing base (a99cfb0) to head (7270601).
⚠️ Report is 7 commits behind head on main.

Additional details and impacted files 
@@           Coverage Diff           @@
##             main     #788   +/-   ##
=======================================
  Coverage   91.07%   91.07%           
=======================================
  Files         110      110           
  Lines       20831    20831           
  Branches    20831    20831           
=======================================
  Hits        18971    18971           
  Misses       1487     1487           
  Partials      373      373

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@kurtisc kurtisc marked this pull request as ready for review February 5, 2026 17:51
@github-actions
Copy link

github-actions bot commented Feb 5, 2026 •
edited
Loading

Benchmark results for revision d5dfecb:

Metric Duration TPS
Mean 1.514848413s 26.406
Worst 1.527238887s 26.191
Best 1.497537668s 26.711
Standard Deviation ±7.528413ms ±0.132
Full results
Run Transfers Duration TPS
1 40 1.527205843s 26.192
2 40 1.523385156s 26.257
3 40 1.527238887s 26.191
4 40 1.519515232s 26.324
5 40 1.513595868s 26.427
6 40 1.505910795s 26.562
7 40 1.514987326s 26.403
8 40 1.510588332s 26.480
9 40 1.521657212s 26.287
10 40 1.513913227s 26.422
11 40 1.512521776s 26.446
12 40 1.514248131s 26.416
13 40 1.517750671s 26.355
14 40 1.516208846s 26.382
15 40 1.519309148s 26.328
16 40 1.515415079s 26.395
17 40 1.510624854s 26.479
18 40 1.499640293s 26.673
19 40 1.497537668s 26.711
20 40 1.515713918s 26.390

Compare the results above with those for the default branch.

NSant215
NSant215 reviewed Feb 6, 2026
View reviewed changes
@kurtisc kurtisc requested a review from NSant215 February 6, 2026 11:14
vapourismo
vapourismo reviewed Feb 6, 2026
View reviewed changes
@kurtisc kurtisc requested a review from vapourismo February 6, 2026 13:02
@kurtisc kurtisc force-pushed the kurtis/durable-storage/bench-feature-refactor branch from 0279f4b to e3b2d99 Compare February 6, 2026 13:03
@kurtisc kurtisc mentioned this pull request Feb 10, 2026
Merged
9 tasks
@kurtisc kurtisc marked this pull request as draft February 10, 2026 10:14
@kurtisc
Copy link
Contributor Author

kurtisc commented Feb 10, 2026

Blocked by moving part of the changes to #805

@kurtisc kurtisc force-pushed the kurtis/durable-storage/bench-feature-refactor branch 2 times, most recently from 16aabde to 29d1ff1 Compare February 10, 2026 10:16
@kurtisc kurtisc changed the base branch from main to kurtis/durable-storage/avl-resolver-fix February 10, 2026 10:16
Base automatically changed from kurtis/durable-storage/avl-resolver-fix to main February 10, 2026 11:13
@kurtisc kurtisc force-pushed the kurtis/durable-storage/bench-feature-refactor branch from 29d1ff1 to 3c4e80f Compare February 10, 2026 13:40
@kurtisc kurtisc marked this pull request as ready for review February 10, 2026 13:40
@kurtisc kurtisc force-pushed the kurtis/durable-storage/bench-feature-refactor branch 2 times, most recently from 79b7c03 to 8e9bcd0 Compare February 10, 2026 16:21
@vapourismo vapourismo force-pushed the kurtis/durable-storage/bench-feature-refactor branch from 8e9bcd0 to 78e71a6 Compare February 11, 2026 11:13
@vapourismo vapourismo enabled auto-merge February 11, 2026 11:13
@vapourismo vapourismo added this pull request to the merge queue Feb 11, 2026
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to a conflict with the base branch Feb 11, 2026
@kurtisc @vapourismo
refactor(bench): self dev-dependency for "bench"
5242657 
Makes `octez-riscv-durable-storage` a dev-dependency of itself with
"bench" enabled.

This means:
 - the benchmarks can be run without specifying the "bench" feature
 - the library and benchmarks are compiled with this feature enabled
   when `make check`/`make all`/`cargo test` are run, so failures can be
   caught in normal development.
@kurtisc @vapourismo
fix: bench export repo
101d463
@kurtisc @vapourismo
refactor(dursto): benchmark re-exports/utilities
59b9edc 
Re-exports parts of the `avl` implementation used in tests.
@kurtisc @vapourismo
refactor(dursto): remove unused get_mut
8badabc
@kurtisc @vapourismo
refactor(Merkle): move test-only code
6bbddfa 
Changes some conditionally-enabled methods to be part of the test
module, since there is no intention to make them available outside of
the tests.
@kurtisc @vapourismo
refactor(Merkle): make get test-only
139f37f 
Removes `get` from the AVL tree benchmark.

Moves the `get` implementation to the `test` module.

These functions are only ever going to be used to check the AVL tree
works as intended.
@kurtisc @vapourismo
chore(avl bench): remove unused pub
7270601
@vapourismo vapourismo force-pushed the kurtis/durable-storage/bench-feature-refactor branch from 78e71a6 to 7270601 Compare February 11, 2026 13:01
@vapourismo vapourismo enabled auto-merge February 11, 2026 13:02
@vapourismo vapourismo added this pull request to the merge queue Feb 11, 2026
Merged via the queue into main with commit cb5ae9c Feb 11, 2026
8 checks passed
@vapourismo vapourismo deleted the kurtis/durable-storage/bench-feature-refactor branch February 11, 2026 14:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vapourismo vapourismo vapourismo approved these changes

@NSant215 NSant215 NSant215 approved these changes

@victor-dumitrescu victor-dumitrescu Awaiting requested review from victor-dumitrescu victor-dumitrescu is a code owner

@jobjo jobjo Awaiting requested review from jobjo jobjo is a code owner

@emturner emturner Awaiting requested review from emturner emturner is a code owner

@thomasathorne thomasathorne Awaiting requested review from thomasathorne thomasathorne is a code owner

@zcabter zcabter Awaiting requested review from zcabter zcabter is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@kurtisc @vapourismo @NSant215