-
Notifications
You must be signed in to change notification settings - Fork 121
kimchi: Improve memory usage and parallelism of expr.evaluations
#3127
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
Open
Fizzixnerd
wants to merge
30
commits into
master
Choose a base branch
from
fizzixnerd/expr-bench
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 22 commits
Commits
Show all changes
30 commits
Select commit
Hold shift + click to select a range
1d31dd8
kimchi: benchmark expression framework
Fizzixnerd 4d81a31
kimchi: fix typo
Fizzixnerd 5c27623
kimchi: enable feature flags unconditionally
Fizzixnerd 944c368
kimchi: refactor to use ToPrimitive
Fizzixnerd 03a6594
kimchi: fix typo
Fizzixnerd cfa8d6c
kimchi: remove dyn and replace with impl
Fizzixnerd 2afcd9f
kimchi: always compute evaluations over D8
Fizzixnerd 935d29c
kimchi: Challenge -> Challenges
Fizzixnerd 1c8f5e7
kimchi: add evaluations_iter
Fizzixnerd 01dab68
kimchi: test and bench new evaluations_iter
Fizzixnerd 180ac3f
kimchi: add par_collect to EvaluationsIter
Fizzixnerd 1301abc
kimchi: cargo fmt
Fizzixnerd 8173fe0
kimchi: improved interface to par_collect; clippy
Fizzixnerd 3b53d40
kimchi: switch `evaluations` to new iterator version and resulting de…
Fizzixnerd 444fd1d
kimchi: cargo fmt
Fizzixnerd 9926ccc
kimchi: cargo +nightly fmt
Fizzixnerd a79271e
Remove useless tests
Fizzixnerd fa6dd49
kimchi: clippy appeasement
Fizzixnerd fe8d42c
kimchi: cargo fmt _again_
Fizzixnerd 7a8aace
kimchi: clippy and fmt test_expr.rs
Fizzixnerd 270ab89
update flake.lock
Fizzixnerd b4e61ae
kimchi: compute over different domains
Fizzixnerd e868d3b
kimchi: remove value_ and compute domain better
Fizzixnerd 30303a4
kimchi: fix typo
Fizzixnerd 12a6e23
kimchi: remove unneeded &mut
Fizzixnerd 384faad
kimchi: cargo fmt
Fizzixnerd b2270df
kimchi: Fix rest of tests
Fizzixnerd 4b12394
kimchi: cargo fmt
Fizzixnerd 47f489f
Merge remote-tracking branch 'origin/master' into fizzixnerd/expr-bench
Fizzixnerd 0fa4e82
WIP
Fizzixnerd File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,80 @@ | ||
use criterion::{criterion_group, criterion_main, Criterion}; | ||
use kimchi::linearization::constraints_expr; | ||
use std::{collections::HashMap, hint::black_box}; | ||
|
||
use ark_ff::UniformRand; | ||
use ark_poly::{Evaluations, Radix2EvaluationDomain as D}; | ||
use kimchi::{ | ||
circuits::{ | ||
berkeley_columns::{BerkeleyChallenges, Environment, E}, | ||
domains::EvaluationDomains, | ||
expr::Constants, | ||
}, | ||
curve::KimchiCurve, | ||
}; | ||
use mina_curves::pasta::{Fp, Vesta}; | ||
use rand::Rng; | ||
|
||
fn create_random_evaluation(domain: D<Fp>, rng: &mut impl Rng) -> Evaluations<Fp, D<Fp>> { | ||
let evals = (0..domain.size).map(|_| Fp::rand(rng)).collect::<Vec<_>>(); | ||
Evaluations::from_vec_and_domain(evals, domain) | ||
} | ||
|
||
fn benchmark_expr_evaluations(c: &mut Criterion) { | ||
let domains = EvaluationDomains::<Fp>::create(1 << 16).unwrap(); | ||
let domain = domains.d8; | ||
let mut rng = rand::thread_rng(); | ||
let randomized_witness = (0..15) | ||
.map(|_| create_random_evaluation(domain, &mut rng)) | ||
.collect::<Vec<_>>() | ||
.try_into() | ||
.unwrap(); | ||
let randomized_coefficients = (0..15) | ||
.map(|_| create_random_evaluation(domain, &mut rng)) | ||
.collect::<Vec<_>>() | ||
.try_into() | ||
.unwrap(); | ||
let randomized_vanishes_on_zero_knowledge_and_previous_rows = | ||
create_random_evaluation(domain, &mut rng); | ||
let randomized_z = create_random_evaluation(domain, &mut rng); | ||
let randomized_l0_1 = Fp::rand(&mut rng); | ||
let constants = Constants { | ||
endo_coefficient: Fp::rand(&mut rng), | ||
mds: &Vesta::sponge_params().mds, | ||
zk_rows: 0, | ||
}; | ||
let challenges = BerkeleyChallenges { | ||
alpha: Fp::rand(&mut rng), | ||
beta: Fp::rand(&mut rng), | ||
gamma: Fp::rand(&mut rng), | ||
joint_combiner: Fp::rand(&mut rng), | ||
}; | ||
|
||
let env = Environment { | ||
witness: &randomized_witness, | ||
coefficient: &randomized_coefficients, | ||
vanishes_on_zero_knowledge_and_previous_rows: | ||
&randomized_vanishes_on_zero_knowledge_and_previous_rows, | ||
z: &randomized_z, | ||
index: HashMap::new(), | ||
l0_1: randomized_l0_1, | ||
constants, | ||
challenges, | ||
domain: domains, | ||
lookup: None, | ||
}; | ||
|
||
let expr: E<Fp> = constraints_expr(None, true).0; | ||
|
||
c.bench_function("expr_evals_par", |b| { | ||
b.iter(|| black_box(expr.clone()).evaluations(black_box(&env))); | ||
}); | ||
} | ||
|
||
criterion_group!( | ||
name = evaluation_bench_seq; | ||
config = Criterion::default().sample_size(10); | ||
targets = benchmark_expr_evaluations | ||
); | ||
|
||
criterion_main!(evaluation_bench_seq); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
This file, in addition to the change in
kimchi/Cargo.toml
, can be extracted from this PR, gathered in a single commit, and provide a good starting point for the changes that you try to introduce in this patch.A reviewer could go after that on the next commits and gradually use the commands you recommend to use to confirm your claims.
In addition to that, it follows the engineering practices we try to enforce. For instance, this practice has been followed in this PR, and it ended up very useful to have atomic commits compiling and passing the whole CI when we had to revert commits.
It seems you started doing in the first commit, but there are comments that you're removing later. Would you mind trying to squash the different commits touching this file to get a clean first commit?