Skip to content

chore: add deferred persistence benchmark#1670

Merged
RodrigoVillar merged 1 commit intomainfrom
rodrigo/add-defer-persist-benchmark
Feb 16, 2026
Merged

chore: add deferred persistence benchmark#1670
RodrigoVillar merged 1 commit intomainfrom
rodrigo/add-defer-persist-benchmark

Conversation

@RodrigoVillar
Copy link
Contributor

@RodrigoVillar RodrigoVillar commented Feb 11, 2026

Why this should be merged

Having benchmarks to test the performance of deferred persistence across different commit counts is useful for observability.

How this works

Adds a benchmark bench_deferred_persistence() to test deferred persistence with commit counts 1 and 100. This benchmark is run daily along with our other benchmarks.

How this was tested

CI

@RodrigoVillar RodrigoVillar force-pushed the rodrigo/async-persist-every-n-commits branch from 25a16aa to 461d452 Compare February 11, 2026 19:04
@RodrigoVillar RodrigoVillar force-pushed the rodrigo/add-defer-persist-benchmark branch from ec8b9c6 to 56e9247 Compare February 11, 2026 19:12
@RodrigoVillar RodrigoVillar force-pushed the rodrigo/async-persist-every-n-commits branch from 461d452 to f81e3ea Compare February 11, 2026 22:12
@RodrigoVillar RodrigoVillar force-pushed the rodrigo/add-defer-persist-benchmark branch 2 times, most recently from e8bcb7d to 558e548 Compare February 12, 2026 13:11
@RodrigoVillar RodrigoVillar force-pushed the rodrigo/async-persist-every-n-commits branch 2 times, most recently from f044ec9 to e2d972f Compare February 12, 2026 13:23
@RodrigoVillar RodrigoVillar force-pushed the rodrigo/add-defer-persist-benchmark branch from 558e548 to 73c824e Compare February 12, 2026 13:23
@RodrigoVillar RodrigoVillar force-pushed the rodrigo/async-persist-every-n-commits branch from e2d972f to 58d5bb3 Compare February 12, 2026 13:40
@RodrigoVillar RodrigoVillar force-pushed the rodrigo/add-defer-persist-benchmark branch 2 times, most recently from 5bcc282 to dd3d85f Compare February 12, 2026 13:52
@RodrigoVillar RodrigoVillar force-pushed the rodrigo/async-persist-every-n-commits branch 3 times, most recently from 82dd5fd to e1e70ce Compare February 13, 2026 15:23
@RodrigoVillar RodrigoVillar force-pushed the rodrigo/add-defer-persist-benchmark branch from dd3d85f to eea5be4 Compare February 13, 2026 15:33
@RodrigoVillar RodrigoVillar force-pushed the rodrigo/async-persist-every-n-commits branch from e1e70ce to d54468a Compare February 13, 2026 20:51
@RodrigoVillar RodrigoVillar force-pushed the rodrigo/add-defer-persist-benchmark branch from eea5be4 to 3dcbcc6 Compare February 13, 2026 20:53
Base automatically changed from rodrigo/async-persist-every-n-commits to main February 13, 2026 21:00
@RodrigoVillar RodrigoVillar force-pushed the rodrigo/add-defer-persist-benchmark branch from 3dcbcc6 to 25321b5 Compare February 13, 2026 21:07
@RodrigoVillar RodrigoVillar marked this pull request as ready for review February 13, 2026 21:08
fn bench_deferred_persistence<const N: usize, const COMMIT_COUNT: u64>(criterion: &mut Criterion) {
const KEY_LEN: usize = 4;
let rng = &firewood_storage::SeededRng::from_option(Some(1234));
let commit_count = NonZeroU64::new(COMMIT_COUNT).unwrap();
Copy link
Member

Choose a reason for hiding this comment

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

super nit: nonzero macro would have worked here since it's a compile time constant.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately, applying the nonzero macro here fails as seen below:

generic parameters may not be used in const operations
const parameters may only be used as standalone arguments here, i.e. `COMMIT_COUNT`

criterion_group! {
name = benches;
config = Criterion::default();
targets = bench_deferred_persistence::<1_000, 1>, bench_deferred_persistence::<1_000, 100>
Copy link
Member

Choose a reason for hiding this comment

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

nit: comment explaining why 1, 100, and 1000 was chosen. I realize we never did this earlier, and maybe there aren't any good reasons :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I chose these values on the fly. However, this does call out a good point - these benchmarks could do a better job in showing the performance gain (or loss) from choosing different commit counts. I went ahead and added more benchmark targets to get a performance curve from persisting every commit to persisting just once.

@RodrigoVillar RodrigoVillar force-pushed the rodrigo/add-defer-persist-benchmark branch from 25321b5 to 2ecddcb Compare February 16, 2026 15:06
@RodrigoVillar RodrigoVillar enabled auto-merge (squash) February 16, 2026 15:09
@RodrigoVillar RodrigoVillar merged commit 7d4d171 into main Feb 16, 2026
51 checks passed
@RodrigoVillar RodrigoVillar deleted the rodrigo/add-defer-persist-benchmark branch February 16, 2026 15:13
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