From db1462b65d60a4f57290801e28094da486b5539f Mon Sep 17 00:00:00 2001 From: ValarDragon Date: Sun, 3 Jan 2021 15:17:53 -0600 Subject: [PATCH 1/5] Reduce symbolic LC depth for mul by constant --- relations/src/r1cs/constraint_system.rs | 45 +++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/relations/src/r1cs/constraint_system.rs b/relations/src/r1cs/constraint_system.rs index c001985d7..3e080c6e9 100644 --- a/relations/src/r1cs/constraint_system.rs +++ b/relations/src/r1cs/constraint_system.rs @@ -213,6 +213,51 @@ impl ConstraintSystem { Ok(var) } + /// Optimized routine for multiplying a variable by a constant `other`. + #[inline] + pub fn mul_var_by_constant( + &mut self, + var: &Variable, + other: &F, + ) -> crate::r1cs::Result { + // Create a new symbolic LC using this constant + #[inline] + fn base_mul_by_constant( + cs: &mut ConstraintSystem, + var: &Variable, + other: F, + ) -> crate::r1cs::Result { + let variable = cs.new_lc(lc!() + (other, *var)).unwrap(); + Ok(variable) + } + + return match var { + Variable::Zero => Ok(Variable::Zero), + Variable::One => base_mul_by_constant(self, var, *other), + Variable::Instance(_) => base_mul_by_constant(self, var, *other), + Variable::Witness(_) => base_mul_by_constant(self, var, *other), + Variable::SymbolicLc(index) => { + // If the underlying symbolic lc only has one element, + // don't create another nested symbolic LC. + // Instead re-use the symbolic LC underlying this variable. + let mut new_lc = lc!(); + { + let lc = self.lc_map.get(&index).unwrap(); + // Case that we make a non-nested symbolic LC + if lc.len() == 1 { + new_lc += (*other * lc[0].0, lc[0].1); + } else { + // Make a nested symbolic LC + // TODO: Under optimization target none, make this non-nested. + return base_mul_by_constant(self, var, *other); + } + } + let variable = self.new_lc(new_lc).unwrap(); + Ok(variable) + } + }; + } + /// Enforce a R1CS constraint with the name `name`. #[inline] pub fn enforce_constraint( From 7fca0781c09bc03a2417ffc7e0ad64337893bd3b Mon Sep 17 00:00:00 2001 From: ValarDragon Date: Tue, 5 Jan 2021 10:15:27 -0600 Subject: [PATCH 2/5] Adapt to optimization goals --- relations/src/r1cs/constraint_system.rs | 33 ++++++++++++++----------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/relations/src/r1cs/constraint_system.rs b/relations/src/r1cs/constraint_system.rs index 8c0d3bcee..51a8b1f30 100644 --- a/relations/src/r1cs/constraint_system.rs +++ b/relations/src/r1cs/constraint_system.rs @@ -273,23 +273,28 @@ impl ConstraintSystem { Variable::Instance(_) => base_mul_by_constant(self, var, *other), Variable::Witness(_) => base_mul_by_constant(self, var, *other), Variable::SymbolicLc(index) => { - // If the underlying symbolic lc only has one element, - // don't create another nested symbolic LC. - // Instead re-use the symbolic LC underlying this variable. - let mut new_lc = lc!(); - { + if self.optimization_goal == OptimizationGoal::Constraints || + self.optimization_goal == OptimizationGoal::None { + // In this case we are optimizing for the number of constraints, + // so we Inline this multiplication by a constant immediately. let lc = self.lc_map.get(&index).unwrap(); - // Case that we make a non-nested symbolic LC - if lc.len() == 1 { - new_lc += (*other * lc[0].0, lc[0].1); - } else { - // Make a nested symbolic LC - // TODO: Under optimization target none, make this non-nested. - return base_mul_by_constant(self, var, *other); + let mut new_lc = lc.clone(); + for i in 0..new_lc.len() { + new_lc[i].0 *= other; } + let variable = self.new_lc(new_lc).unwrap(); + return Ok(variable); + } + else { + // In this case we are optimizing for constraint density, + // so we want to know when a symbolic LC is being re-used. + base_mul_by_constant(self, var, *other) + // TODO: If `var` is a symbolic LC with a single element, + // we don't need to make the new symbolic LC nest on top of var. + // It can instead re-use the single variable underlying `var`. + // This costs us an extra lookup though, so we should only do such + // a switch if benchmarks prove it to be useful. } - let variable = self.new_lc(new_lc).unwrap(); - Ok(variable) } }; } From ca93febd3451d3d7753c37186632fdd9cfdc16a7 Mon Sep 17 00:00:00 2001 From: ValarDragon Date: Tue, 5 Jan 2021 10:26:25 -0600 Subject: [PATCH 3/5] Fix lint, add changelog --- CHANGELOG.md | 1 + relations/src/r1cs/constraint_system.rs | 8 ++++---- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bce0b6613..2a318611a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ ### Improvements - #325 Reduce memory consumption during inlining +- #339 Reduce memory consumption under `Constraints` optimization goal ### Bug fixes diff --git a/relations/src/r1cs/constraint_system.rs b/relations/src/r1cs/constraint_system.rs index 51a8b1f30..4310f4860 100644 --- a/relations/src/r1cs/constraint_system.rs +++ b/relations/src/r1cs/constraint_system.rs @@ -273,8 +273,9 @@ impl ConstraintSystem { Variable::Instance(_) => base_mul_by_constant(self, var, *other), Variable::Witness(_) => base_mul_by_constant(self, var, *other), Variable::SymbolicLc(index) => { - if self.optimization_goal == OptimizationGoal::Constraints || - self.optimization_goal == OptimizationGoal::None { + if self.optimization_goal == OptimizationGoal::Constraints + || self.optimization_goal == OptimizationGoal::None + { // In this case we are optimizing for the number of constraints, // so we Inline this multiplication by a constant immediately. let lc = self.lc_map.get(&index).unwrap(); @@ -284,8 +285,7 @@ impl ConstraintSystem { } let variable = self.new_lc(new_lc).unwrap(); return Ok(variable); - } - else { + } else { // In this case we are optimizing for constraint density, // so we want to know when a symbolic LC is being re-used. base_mul_by_constant(self, var, *other) From 13e01a71ec023633fe8009c3e678e9053fcad13d Mon Sep 17 00:00:00 2001 From: ValarDragon Date: Tue, 5 Jan 2021 10:30:39 -0600 Subject: [PATCH 4/5] More comments --- relations/src/r1cs/constraint_system.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/relations/src/r1cs/constraint_system.rs b/relations/src/r1cs/constraint_system.rs index 4310f4860..9129dcca9 100644 --- a/relations/src/r1cs/constraint_system.rs +++ b/relations/src/r1cs/constraint_system.rs @@ -256,7 +256,8 @@ impl ConstraintSystem { var: &Variable, other: &F, ) -> crate::r1cs::Result { - // Create a new symbolic LC using this constant + // Create a new symbolic LC using this constant, deffering the inlining + // until the finalize phase #[inline] fn base_mul_by_constant( cs: &mut ConstraintSystem, @@ -277,7 +278,8 @@ impl ConstraintSystem { || self.optimization_goal == OptimizationGoal::None { // In this case we are optimizing for the number of constraints, - // so we Inline this multiplication by a constant immediately. + // so we have no need for symbolic variables of intermediate computation steps. + // Thus we inline this multiplication by a constant immediately. let lc = self.lc_map.get(&index).unwrap(); let mut new_lc = lc.clone(); for i in 0..new_lc.len() { From 543eedb53d2dbd9cfc91a917b7be9ebdaf682759 Mon Sep 17 00:00:00 2001 From: Weikeng Chen Date: Tue, 5 Jan 2021 09:50:44 -0800 Subject: [PATCH 5/5] minor --- relations/src/r1cs/constraint_system.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/relations/src/r1cs/constraint_system.rs b/relations/src/r1cs/constraint_system.rs index 9129dcca9..03c5d1073 100644 --- a/relations/src/r1cs/constraint_system.rs +++ b/relations/src/r1cs/constraint_system.rs @@ -256,8 +256,8 @@ impl ConstraintSystem { var: &Variable, other: &F, ) -> crate::r1cs::Result { - // Create a new symbolic LC using this constant, deffering the inlining - // until the finalize phase + // Create a new symbolic LC using this constant, deferring the inlining + // until the finalization phase. #[inline] fn base_mul_by_constant( cs: &mut ConstraintSystem,