Merge two similar benchmarks in BaseFold into one #554
Conversation
yczhangsjtu
changed the title
matthiasgoergens
changed the title
mpcs/src/lib.rs
Outdated
| // unfortunately integration benchmarks do not compile the #[cfg(test)] | ||
| // code. So remove the gate for the entire module, only gate the test | ||
| // functions. | ||
| // TODO(Matthias): use cfg(or(test, bench)) or something like that. |
There was a problem hiding this comment.
We might want to actually do this. :) Let me see what I can do for you. (Or otherwise, remove my TODO-comment from the PR.)
| } | ||
| pub fn gen_rand_poly_base<E: ExtensionField>(num_vars: usize) -> DenseMultilinearExtension<E> { | ||
| DenseMultilinearExtension::random(num_vars, &mut OsRng) | ||
| } |
There was a problem hiding this comment.
in lib code (instead of testing code), we usually don't hard code RNG directly. (besides, OsRng is slow actually, most of the time we need some kind of lightweighted rng like CHACHA RNG, that would be enough.
So i suggest give user of this function the freedom to choose rng , and it is ok to use osrng in testing codes)
| // functions. | ||
| // This is not the best way: the test utility functions should not be | ||
| // compiled in the release build. Need a better solution. | ||
| #[doc(hidden)] |
There was a problem hiding this comment.
in zkevm-circuits, we have a "test" feature flag. and we write "#[cfg(any(feature = "test", test))]" a lot of places.
There was a problem hiding this comment.
That means we run benchmarks using cargo bench --features "test"?
Extracting small PR from #294.
The two benchmarks
benches_commit_verify_rsandbenches_commit_verify_basecodeare similar, differing only in the choice of parameter. Merge them into one benchmark file.Also simplify the codes using the test utility functions introduced in #552
Waiting for #552 to merge.