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

Remove redundant code and prevent race conditions #19

Merged
merged 3 commits into from
Apr 16, 2023
Merged

Remove redundant code and prevent race conditions #19

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a6de104
feat: move `GateCircuitBuilder::synthesize` to `sub_synthesize` function
jonathanpwang Mar 30, 2023
febfc94
feat: remove `Mutex` to prevent non-deterministism
jonathanpwang Apr 8, 2023
59194f0
fix: `fixed_base::msm_par` loading new zeros
jonathanpwang Apr 8, 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
159 changes: 67 additions & 92 deletions halo2-base/src/gates/builder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -409,6 +409,69 @@ impl<F: ScalarField> GateCircuitBuilder<F> {
) -> Self {
Self { builder: RefCell::new(builder), break_points: RefCell::new(break_points) }
}

pub fn sub_synthesize(
&self,
gate: &FlexGateConfig<F>,
lookup_advice: &[Vec<Column<Advice>>],
q_lookup: &[Option<Selector>],
layouter: &mut impl Layouter<F>,
) -> HashMap<(usize, usize), (circuit::Cell, usize)> {
let mut first_pass = SKIP_FIRST_PASS;
let mut assigned_advices = HashMap::new();
layouter
.assign_region(
|| "GateCircuitBuilder generated circuit",
|mut region| {
if first_pass {
first_pass = false;
return Ok(());
}
// only support FirstPhase in this Builder because getting challenge value requires more specialized witness generation during synthesize
if !self.builder.borrow().witness_gen_only {
// clone the builder so we can re-use the circuit for both vk and pk gen
let builder = self.builder.borrow().clone();
for threads in builder.threads.iter().skip(1) {
assert!(
threads.is_empty(),
"GateCircuitBuilder only supports FirstPhase for now"
);
}
let assignments = builder.assign_all(
gate,
lookup_advice,
q_lookup,
&mut region,
Default::default(),
);
*self.break_points.borrow_mut() = assignments.break_points;
assigned_advices = assignments.assigned_advices;
} else {
let builder = self.builder.take();
let break_points = self.break_points.take();
for (phase, (threads, break_points)) in builder
.threads
.into_iter()
.zip(break_points.into_iter())
.enumerate()
.take(1)
{
assign_threads_in(
phase,
threads,
gate,
lookup_advice.get(phase).unwrap_or(&vec![]),
&mut region,
break_points,
);
}
}
Ok(())
},
)
.unwrap();
assigned_advices
}
}

impl<F: ScalarField> Circuit<F> for GateCircuitBuilder<F> {
Expand All @@ -435,43 +498,8 @@ impl<F: ScalarField> Circuit<F> for GateCircuitBuilder<F> {
config: Self::Config,
mut layouter: impl Layouter<F>,
) -> Result<(), Error> {
let mut first_pass = SKIP_FIRST_PASS;
layouter.assign_region(
|| "GateCircuitBuilder generated circuit",
|mut region| {
if first_pass {
first_pass = false;
return Ok(());
}
// only support FirstPhase in this Builder because getting challenge value requires more specialized witness generation during synthesize
if !self.builder.borrow().witness_gen_only {
// clone the builder so we can re-use the circuit for both vk and pk gen
let builder = self.builder.borrow().clone();
for threads in builder.threads.iter().skip(1) {
assert!(
threads.is_empty(),
"GateCircuitBuilder only supports FirstPhase for now"
);
}
*self.break_points.borrow_mut() = builder
.assign_all(&config, &[], &[], &mut region, Default::default())
.break_points;
} else {
let builder = self.builder.take();
let break_points = self.break_points.take();
for (phase, (threads, break_points)) in builder
.threads
.into_iter()
.zip(break_points.into_iter())
.enumerate()
.take(1)
{
assign_threads_in(phase, threads, &config, &[], &mut region, break_points);
}
}
Ok(())
},
)
self.sub_synthesize(&config, &[], &[], &mut layouter);
Ok(())
}
}

Expand Down Expand Up @@ -533,61 +561,8 @@ impl<F: ScalarField> Circuit<F> for RangeCircuitBuilder<F> {
mut layouter: impl Layouter<F>,
) -> Result<(), Error> {
config.load_lookup_table(&mut layouter).expect("load lookup table should not fail");

let mut first_pass = SKIP_FIRST_PASS;
layouter.assign_region(
|| "RangeCircuitBuilder generated circuit",
|mut region| {
if first_pass {
first_pass = false;
return Ok(());
}
// only support FirstPhase in this Builder because getting challenge value requires more specialized witness generation during synthesize
if !self.0.builder.borrow().witness_gen_only {
// clone the builder so we can re-use the circuit for both vk and pk gen
let builder = self.0.builder.borrow().clone();
for threads in builder.threads.iter().skip(1) {
assert!(
threads.is_empty(),
"GateCircuitBuilder only supports FirstPhase for now"
);
}
*self.0.break_points.borrow_mut() = builder
.assign_all(
&config.gate,
&config.lookup_advice,
&config.q_lookup,
&mut region,
Default::default(),
)
.break_points;
} else {
#[cfg(feature = "display")]
let start0 = std::time::Instant::now();
let builder = self.0.builder.take();
let break_points = self.0.break_points.take();
for (phase, (threads, break_points)) in builder
.threads
.into_iter()
.zip(break_points.into_iter())
.enumerate()
.take(1)
{
assign_threads_in(
phase,
threads,
&config.gate,
&config.lookup_advice[phase],
&mut region,
break_points,
);
}
#[cfg(feature = "display")]
println!("assign threads in {:?}", start0.elapsed());
}
Ok(())
},
)
self.0.sub_synthesize(&config.gate, &config.lookup_advice, &config.q_lookup, &mut layouter);
Ok(())
}
}

Expand Down
13 changes: 4 additions & 9 deletions halo2-ecc/benches/fixed_base_msm.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ use halo2_base::halo2_proofs::{
};
use halo2_ecc::{bn254::FpChip, ecc::EccChip, fields::PrimeField};
use rand::rngs::OsRng;
use std::sync::Mutex;

use criterion::{criterion_group, criterion_main};
use criterion::{BenchmarkId, Criterion};
Expand All @@ -41,7 +40,7 @@ const BEST_100_CONFIG: MSMCircuitParams =
const TEST_CONFIG: MSMCircuitParams = BEST_100_CONFIG;

fn fixed_base_msm_bench(
thread_pool: &Mutex<GateThreadBuilder<Fr>>,
builder: &mut GateThreadBuilder<Fr>,
params: MSMCircuitParams,
bases: Vec<G1Affine>,
scalars: Vec<Fr>,
Expand All @@ -51,14 +50,12 @@ fn fixed_base_msm_bench(
let fp_chip = FpChip::<Fr>::new(&range, params.limb_bits, params.num_limbs);
let ecc_chip = EccChip::new(&fp_chip);

let mut builder = thread_pool.lock().unwrap();
let scalars_assigned = scalars
.iter()
.map(|scalar| vec![builder.main(0).load_witness(*scalar)])
.collect::<Vec<_>>();
drop(builder);

ecc_chip.fixed_base_msm(thread_pool, &bases, scalars_assigned, Fr::NUM_BITS as usize);
ecc_chip.fixed_base_msm(builder, &bases, scalars_assigned, Fr::NUM_BITS as usize);
}

fn fixed_base_msm_circuit(
Expand All @@ -69,17 +66,15 @@ fn fixed_base_msm_circuit(
break_points: Option<MultiPhaseThreadBreakPoints>,
) -> RangeCircuitBuilder<Fr> {
let k = params.degree as usize;
let builder = match stage {
let mut builder = match stage {
CircuitBuilderStage::Mock => GateThreadBuilder::mock(),
CircuitBuilderStage::Prover => GateThreadBuilder::prover(),
CircuitBuilderStage::Keygen => GateThreadBuilder::keygen(),
};
let builder = Mutex::new(builder);

let start0 = start_timer!(|| format!("Witness generation for circuit in {stage:?} stage"));
fixed_base_msm_bench(&builder, params, bases, scalars);
fixed_base_msm_bench(&mut builder, params, bases, scalars);

let builder = builder.into_inner().unwrap();
let circuit = match stage {
CircuitBuilderStage::Mock => {
builder.config(k, Some(20));
Expand Down
13 changes: 4 additions & 9 deletions halo2-ecc/benches/msm.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ use halo2_base::halo2_proofs::{
};
use halo2_ecc::{bn254::FpChip, ecc::EccChip, fields::PrimeField};
use rand::rngs::OsRng;
use std::sync::Mutex;

use criterion::{criterion_group, criterion_main};
use criterion::{BenchmarkId, Criterion};
Expand Down Expand Up @@ -47,7 +46,7 @@ const BEST_100_CONFIG: MSMCircuitParams = MSMCircuitParams {
const TEST_CONFIG: MSMCircuitParams = BEST_100_CONFIG;

fn msm_bench(
thread_pool: &Mutex<GateThreadBuilder<Fr>>,
builder: &mut GateThreadBuilder<Fr>,
params: MSMCircuitParams,
bases: Vec<G1Affine>,
scalars: Vec<Fr>,
Expand All @@ -57,16 +56,14 @@ fn msm_bench(
let fp_chip = FpChip::<Fr>::new(&range, params.limb_bits, params.num_limbs);
let ecc_chip = EccChip::new(&fp_chip);

let mut builder = thread_pool.lock().unwrap();
let ctx = builder.main(0);
let scalars_assigned =
scalars.iter().map(|scalar| vec![ctx.load_witness(*scalar)]).collect::<Vec<_>>();
let bases_assigned =
bases.iter().map(|base| ecc_chip.load_private(ctx, (base.x, base.y))).collect::<Vec<_>>();
drop(builder);

ecc_chip.variable_base_msm_in::<G1Affine>(
thread_pool,
builder,
&bases_assigned,
scalars_assigned,
Fr::NUM_BITS as usize,
Expand All @@ -84,16 +81,14 @@ fn msm_circuit(
) -> RangeCircuitBuilder<Fr> {
let start0 = start_timer!(|| format!("Witness generation for circuit in {stage:?} stage"));
let k = params.degree as usize;
let builder = match stage {
let mut builder = match stage {
CircuitBuilderStage::Mock => GateThreadBuilder::mock(),
CircuitBuilderStage::Prover => GateThreadBuilder::prover(),
CircuitBuilderStage::Keygen => GateThreadBuilder::keygen(),
};
let builder = Mutex::new(builder);

msm_bench(&builder, params, bases, scalars);
msm_bench(&mut builder, params, bases, scalars);

let builder = builder.into_inner().unwrap();
let circuit = match stage {
CircuitBuilderStage::Mock => {
builder.config(k, Some(20));
Expand Down
13 changes: 4 additions & 9 deletions halo2-ecc/src/bn254/tests/fixed_base_msm.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use std::{
fs::{self, File},
io::{BufRead, BufReader},
sync::Mutex,
};

#[allow(unused_imports)]
Expand Down Expand Up @@ -40,7 +39,7 @@ struct MSMCircuitParams {
}

fn fixed_base_msm_test(
thread_pool: &Mutex<GateThreadBuilder<Fr>>,
builder: &mut GateThreadBuilder<Fr>,
params: MSMCircuitParams,
bases: Vec<G1Affine>,
scalars: Vec<Fr>,
Expand All @@ -50,14 +49,12 @@ fn fixed_base_msm_test(
let fp_chip = FpChip::<Fr>::new(&range, params.limb_bits, params.num_limbs);
let ecc_chip = EccChip::new(&fp_chip);

let mut builder = thread_pool.lock().unwrap();
let scalars_assigned = scalars
.iter()
.map(|scalar| vec![builder.main(0).load_witness(*scalar)])
.collect::<Vec<_>>();
drop(builder);

let msm = ecc_chip.fixed_base_msm(thread_pool, &bases, scalars_assigned, Fr::NUM_BITS as usize);
let msm = ecc_chip.fixed_base_msm(builder, &bases, scalars_assigned, Fr::NUM_BITS as usize);

let mut elts: Vec<G1> = Vec::new();
for (base, scalar) in bases.iter().zip(scalars.iter()) {
Expand All @@ -77,19 +74,17 @@ fn random_fixed_base_msm_circuit(
break_points: Option<MultiPhaseThreadBreakPoints>,
) -> RangeCircuitBuilder<Fr> {
let k = params.degree as usize;
let builder = match stage {
let mut builder = match stage {
CircuitBuilderStage::Mock => GateThreadBuilder::mock(),
CircuitBuilderStage::Prover => GateThreadBuilder::prover(),
CircuitBuilderStage::Keygen => GateThreadBuilder::keygen(),
};
let builder = Mutex::new(builder);

let (bases, scalars): (Vec<_>, Vec<_>) =
(0..params.batch_size).map(|_| (G1Affine::random(OsRng), Fr::random(OsRng))).unzip();
let start0 = start_timer!(|| format!("Witness generation for circuit in {stage:?} stage"));
fixed_base_msm_test(&builder, params, bases, scalars);
fixed_base_msm_test(&mut builder, params, bases, scalars);

let builder = builder.into_inner().unwrap();
let circuit = match stage {
CircuitBuilderStage::Mock => {
builder.config(k, Some(20));
Expand Down
13 changes: 4 additions & 9 deletions halo2-ecc/src/bn254/tests/msm.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ use rand_core::OsRng;
use std::{
fs::{self, File},
io::{BufRead, BufReader},
sync::Mutex,
};

use super::*;
Expand All @@ -34,7 +33,7 @@ struct MSMCircuitParams {
}

fn msm_test(
thread_pool: &Mutex<GateThreadBuilder<Fr>>,
builder: &mut GateThreadBuilder<Fr>,
params: MSMCircuitParams,
bases: Vec<G1Affine>,
scalars: Vec<Fr>,
Expand All @@ -45,16 +44,14 @@ fn msm_test(
let fp_chip = FpChip::<Fr>::new(&range, params.limb_bits, params.num_limbs);
let ecc_chip = EccChip::new(&fp_chip);

let mut builder = thread_pool.lock().unwrap();
let ctx = builder.main(0);
let scalars_assigned =
scalars.iter().map(|scalar| vec![ctx.load_witness(*scalar)]).collect::<Vec<_>>();
let bases_assigned =
bases.iter().map(|base| ecc_chip.load_private(ctx, (base.x, base.y))).collect::<Vec<_>>();
drop(builder);

let msm = ecc_chip.variable_base_msm_in::<G1Affine>(
thread_pool,
builder,
&bases_assigned,
scalars_assigned,
Fr::NUM_BITS as usize,
Expand Down Expand Up @@ -82,19 +79,17 @@ fn random_msm_circuit(
break_points: Option<MultiPhaseThreadBreakPoints>,
) -> RangeCircuitBuilder<Fr> {
let k = params.degree as usize;
let builder = match stage {
let mut builder = match stage {
CircuitBuilderStage::Mock => GateThreadBuilder::mock(),
CircuitBuilderStage::Prover => GateThreadBuilder::prover(),
CircuitBuilderStage::Keygen => GateThreadBuilder::keygen(),
};
let builder = Mutex::new(builder);

let (bases, scalars): (Vec<_>, Vec<_>) =
(0..params.batch_size).map(|_| (G1Affine::random(OsRng), Fr::random(OsRng))).unzip();
let start0 = start_timer!(|| format!("Witness generation for circuit in {stage:?} stage"));
msm_test(&builder, params, bases, scalars, params.window_bits);
msm_test(&mut builder, params, bases, scalars, params.window_bits);

let builder = builder.into_inner().unwrap();
let circuit = match stage {
CircuitBuilderStage::Mock => {
builder.config(k, Some(20));
Expand Down
Loading