Skip to content
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.

Sign up for GitHub

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

Jump to bottom

chore: Speed up random poly generation #742

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

chore: Speed up random poly generation #742

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3689925
chore: Speed up random poly generation
CPerezz Feb 27, 2023
e49cf60
fix: Split RngCore and SeedableRng imports
CPerezz Feb 27, 2023
a4cc9f6
fix: Upstream concurrent poly generation fixes
CPerezz Feb 28, 2023
580d982
chore: Update `k` to be obtained from struct
CPerezz Feb 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion halo2_proofs/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ rand_core = { version = "0.6", default-features = false }
tracing = "0.1"
blake2b_simd = "1"
maybe-rayon = {version = "0.1.0", default-features = false}
rand_chacha = { version = "0.3", optional = true }

# Developer tooling dependencies
plotters = { version = "0.3.0", default-features = false, optional = true }
Expand All @@ -69,7 +70,7 @@ getrandom = { version = "0.2", features = ["js"] }

[features]
default = ["batch", "multicore"]
multicore = ["maybe-rayon/threads"]
multicore = ["maybe-rayon/threads", "rand_chacha"]
dev-graph = ["plotters", "tabbycat"]
test-dev-graph = [
"dev-graph",
Expand Down
48 changes: 44 additions & 4 deletions halo2_proofs/src/plonk/vanishing/prover.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,13 @@ use ff::Field;
use group::Curve;
use rand_core::RngCore;

#[cfg(feature = "multicore")]
use maybe_rayon::{current_num_threads, prelude::*};
#[cfg(feature = "multicore")]
use rand_chacha::ChaCha20Rng;
#[cfg(feature = "multicore")]
use rand_core::SeedableRng;

use super::Argument;
use crate::{
arithmetic::{eval_polynomial, CurveAffine},
Expand Down Expand Up @@ -42,10 +49,43 @@ impl<C: CurveAffine> Argument<C> {
transcript: &mut T,
) -> Result<Committed<C>, Error> {
// Sample a random polynomial of degree n - 1
let mut random_poly = domain.empty_coeff();
for coeff in random_poly.iter_mut() {
*coeff = C::Scalar::random(&mut rng);
}
#[cfg(feature = "multicore")]
let random_poly = {
Comment on lines -45 to +53
Copy link
Contributor

Choose a reason for hiding this comment

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

Rather than expanding this inline here, we should just add an EvaluationDomain::random_coeff<R: RngCore>(&self, rng: R) method. This would also avoid the need to make EvaluationDomain.k pub(crate).

let n_threads = current_num_threads();
let n = 1usize << domain.k as usize;
Copy link
Contributor

Choose a reason for hiding this comment

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

Moving this to EvaluationDomain::random_coeff would also mean we can use self.n instead of recomputing it here:

Suggested change
let n = 1usize << domain.k as usize;

let n_chunks = n_threads + if n % n_threads != 0 { 1 } else { 0 };
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
let n_chunks = n_threads + if n % n_threads != 0 { 1 } else { 0 };
let chunk_size = (self.n + n_threads - 1) / n_threads;
let num_chunks = (self.n + chunk_size - 1) / chunk_size;

let mut rand_vec = vec![C::Scalar::ZERO; n];

let mut thread_seeds: Vec<ChaCha20Rng> = (0..n_chunks)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
let mut thread_seeds: Vec<ChaCha20Rng> = (0..n_chunks)
let mut thread_seeds: Vec<ChaCha20Rng> = (0..num_chunks)

.into_iter()
.map(|_| {
let mut seed = [0u8; 32];
rng.fill_bytes(&mut seed);
ChaCha20Rng::from_seed(seed)
Copy link
Contributor

@daira daira Feb 28, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I believe this could safely use ChaCha12Rng while conservatively meeting our security requirements. The paper Too Much Crypto argues in favour of ChaCha8; I do not entirely endorse all of the conclusions of that paper, but there is no significant doubt in my mind that 12 rounds is sufficient. There is a more recent (Nov 2021) paper on cryptanalysis of ChaCha, PNB-focused Differential Cryptanalysis of ChaCha Stream Cipher, published since the ones considered in Too Much Crypto (which is originally from Dec 2019), but the newer paper does not improve on brute force and in any case would only extend to 7.25 rounds if it did.

})
.collect();

thread_seeds
.par_iter_mut()
.zip_eq(rand_vec.par_chunks_mut(n / n_threads))
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
.zip_eq(rand_vec.par_chunks_mut(n / n_threads))
.zip_eq(rand_vec.par_chunks_mut(chunk_size))

.for_each(|(mut rng, chunk)| {
chunk
.iter_mut()
.for_each(|v| *v = C::Scalar::random(&mut rng))
});

domain.coeff_from_vec(rand_vec)
};

#[cfg(not(feature = "multicore"))]
let random_poly = {
let mut random_poly = domain.empty_coeff();
for coeff in random_poly.iter_mut() {
*coeff = C::Scalar::random(&mut rng);
}
random_poly
};

// Sample a random blinding factor
let random_blind = Blind(C::Scalar::random(rng));

Expand Down
2 changes: 1 addition & 1 deletion halo2_proofs/src/poly/domain.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use std::marker::PhantomData;
#[derive(Clone, Debug)]
pub struct EvaluationDomain<F: Field> {
n: u64,
k: u32,
pub(crate) k: u32,
extended_k: u32,
omega: F,
omega_inv: F,
Expand Down