-
Notifications
You must be signed in to change notification settings - Fork 10
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
Benchmarks related to Share Refresh #41
Conversation
Getting the error: error: unused `ark_ec::short_weierstrass_jacobian::GroupProjective` that must be used
--> tpke/benches/tpke.rs:327:21
|
327 | / black_box(recover_share_at_point::<E>(
328 | | &remaining_participants[..shares_num],
[329](https://github.com/nucypher/ferveo/actions/runs/3968438305/jobs/6801725125#step:5:330) | | threshold,
[330](https://github.com/nucypher/ferveo/actions/runs/3968438305/jobs/6801725125#step:5:331) | | &x_r,
331 | | &mut rng,
332 | | ));
| |______________________^
|
= note: `-D unused-must-use` implied by `-D warnings`
```
but the type that is specifies isn't used in the code |
Co-authored-by: piotr-roslaniec <39299780+piotr-roslaniec@users.noreply.github.com>
tpke/benches/tpke.rs
Outdated
for p in &mut remaining_participants { | ||
p.public_decryption_contexts.pop(); | ||
} | ||
group.bench_with_input( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reason why you've selected bench_with_input
over bench_function
? Did you have any issues with ownership? Just curious because closures always worked fine for me.
tpke/benches/tpke.rs
Outdated
group.bench_with_input( | ||
BenchmarkId::new("Recover Share at Point", shares_num), | ||
&shares_num, | ||
|b, _| { | ||
let mut rng = rand::rngs::StdRng::seed_from_u64(0); | ||
b.iter(|| { | ||
let _ = black_box(recover_share_at_point::<E>( | ||
&remaining_participants[..shares_num], | ||
threshold, | ||
&x_r, | ||
&mut rng, | ||
)); | ||
}); | ||
}, | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you don't need to use bench_with_input
here: Notice that shares_num
passed in as a second argument is unused: |b, _|
group.bench_with_input( | |
BenchmarkId::new("Recover Share at Point", shares_num), | |
&shares_num, | |
|b, _| { | |
let mut rng = rand::rngs::StdRng::seed_from_u64(0); | |
b.iter(|| { | |
let _ = black_box(recover_share_at_point::<E>( | |
&remaining_participants[..shares_num], | |
threshold, | |
&x_r, | |
&mut rng, | |
)); | |
}); | |
}, | |
); | |
group.bench_function( | |
BenchmarkId::new("Recover Share at Point", shares_num), | |
|b| { | |
let mut rng = rand::rngs::StdRng::seed_from_u64(0); | |
b.iter(|| { | |
let _ = black_box(recover_share_at_point::<E>( | |
&remaining_participants[..shares_num], | |
threshold, | |
&x_r, | |
&mut rng, | |
)); | |
}); | |
}, | |
); |
tpke/benches/tpke.rs
Outdated
group.bench_with_input( | ||
BenchmarkId::new("Refresh Shares", shares_num), | ||
&shares_num, | ||
|b, _| { | ||
let mut rng = rand::rngs::StdRng::seed_from_u64(0); | ||
b.iter(|| { | ||
black_box(refresh_shares::<E>( | ||
&mut setup.contexts, | ||
threshold, | ||
&mut rng, | ||
)); | ||
}); | ||
}, | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similarly here:
group.bench_with_input( | |
BenchmarkId::new("Refresh Shares", shares_num), | |
&shares_num, | |
|b, _| { | |
let mut rng = rand::rngs::StdRng::seed_from_u64(0); | |
b.iter(|| { | |
black_box(refresh_shares::<E>( | |
&mut setup.contexts, | |
threshold, | |
&mut rng, | |
)); | |
}); | |
}, | |
); | |
group.bench_function( | |
BenchmarkId::new("Refresh Shares", shares_num), | |
|b| { | |
let mut rng = rand::rngs::StdRng::seed_from_u64(0); | |
b.iter(|| { | |
black_box(refresh_shares::<E>( | |
&mut setup.contexts, | |
threshold, | |
&mut rng, | |
)); | |
}); | |
}, | |
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feel free to address my comments & merge at will
Finishes #24