From 78d7f8c6d34813e54ffb29091e8821d5b334c12e Mon Sep 17 00:00:00 2001 From: Daniel Lubarov Date: Fri, 26 Feb 2021 13:33:05 -0800 Subject: [PATCH] Don't need ConstraintPolynomialRef --- src/constraint_polynomial.rs | 119 ++++++++++++++++------------------- src/gates/output_graph.rs | 2 +- 2 files changed, 54 insertions(+), 67 deletions(-) diff --git a/src/constraint_polynomial.rs b/src/constraint_polynomial.rs index ca048e30..adcedb30 100644 --- a/src/constraint_polynomial.rs +++ b/src/constraint_polynomial.rs @@ -21,12 +21,12 @@ pub(crate) struct EvaluationVars<'a, F: Field> { /// A polynomial over all the variables that are subject to constraints (local constants, next /// constants, local wire values, and next wire values). This representation does not require any /// particular form; it permits arbitrary forms such as `(x + 1)^3 + y z`. -// Implementation note: This is a wrapper because we want to hide complexity behind -// `ConstraintPolynomialInner` and `ConstraintPolynomialRef`. In particular, the caller shouldn't -// need to know that we use reference counting internally, and shouldn't have to deal with wrapper -// types related to reference counting. +/// +/// This type implements `Hash` and `Eq` based on references rather +/// than content. This is useful when we want to use constraint polynomials as `HashMap` keys, but +/// we want address-based hashing for performance reasons. #[derive(Clone)] -pub struct ConstraintPolynomial(ConstraintPolynomialRef); +pub struct ConstraintPolynomial(Rc>); impl ConstraintPolynomial { pub fn constant(c: F) -> Self { @@ -72,8 +72,8 @@ impl ConstraintPolynomial { pub fn add(&self, rhs: &Self) -> Self { // TODO: Special case for either operand being 0. Self::from_inner(ConstraintPolynomialInner::Sum { - lhs: self.0.clone(), - rhs: rhs.0.clone(), + lhs: self.clone(), + rhs: rhs.clone(), }) } @@ -95,14 +95,14 @@ impl ConstraintPolynomial { pub fn mul(&self, rhs: &Self) -> Self { // TODO: Special case for either operand being 1. Self::from_inner(ConstraintPolynomialInner::Product { - lhs: self.0.clone(), - rhs: rhs.0.clone(), + lhs: self.clone(), + rhs: rhs.clone(), }) } pub fn exp(&self, exponent: usize) -> Self { Self::from_inner(ConstraintPolynomialInner::Exponentiation { - base: self.0.clone(), + base: self.clone(), exponent, }) } @@ -116,14 +116,14 @@ impl ConstraintPolynomial { } pub(crate) fn degree(&self) -> BigUint { - (self.0).0.degree() + self.0.degree() } /// Returns the set of wires that this constraint would depend on if it were applied at a /// certain gate index. pub(crate) fn dependencies(&self, gate: usize) -> Vec { let mut deps = HashSet::new(); - self.0.0.add_dependencies(gate, &mut deps); + self.0.add_dependencies(gate, &mut deps); deps.into_iter().collect() } @@ -137,7 +137,7 @@ impl ConstraintPolynomial { pub(crate) fn max_constant_index(&self) -> Option { let mut indices = HashSet::new(); - self.0.0.add_constant_indices(&mut indices); + self.0.add_constant_indices(&mut indices); indices.into_iter().max() } @@ -155,21 +155,49 @@ impl ConstraintPolynomial { ) -> Vec { let mut mem = HashMap::new(); polynomials.iter() - .map(|p| p.0.evaluate_memoized(&vars, &mut mem)) + .map(|p| p.evaluate_memoized(&vars, &mut mem)) .collect() } + fn evaluate_memoized( + &self, + vars: &EvaluationVars, + mem: &mut HashMap, + ) -> F { + if let Some(&result) = mem.get(self) { + result + } else { + let result = self.0.evaluate(vars, mem); + mem.insert(self.clone(), result); + result + } + } + /// Replace all occurrences of `from` with `to` in this polynomial graph. pub(crate) fn replace_all( &self, from: Self, to: Self, ) -> Self { - Self(self.0.replace_all(from.0, to.0)) + Self::from_inner(self.0.replace_all(from, to)) } fn from_inner(inner: ConstraintPolynomialInner) -> Self { - Self(ConstraintPolynomialRef::new(inner)) + Self(Rc::new(inner)) + } +} + +impl PartialEq for ConstraintPolynomial { + fn eq(&self, other: &Self) -> bool { + ptr::eq(&*self.0, &*other.0) + } +} + +impl Eq for ConstraintPolynomial {} + +impl Hash for ConstraintPolynomial { + fn hash(&self, state: &mut H) { + ptr::hash(&*self.0, state); } } @@ -346,15 +374,15 @@ enum ConstraintPolynomialInner { NextWireValue(usize), Sum { - lhs: ConstraintPolynomialRef, - rhs: ConstraintPolynomialRef, + lhs: ConstraintPolynomial, + rhs: ConstraintPolynomial, }, Product { - lhs: ConstraintPolynomialRef, - rhs: ConstraintPolynomialRef, + lhs: ConstraintPolynomial, + rhs: ConstraintPolynomial, }, Exponentiation { - base: ConstraintPolynomialRef, + base: ConstraintPolynomial, exponent: usize, }, } @@ -407,7 +435,7 @@ impl ConstraintPolynomialInner { fn evaluate( &self, vars: &EvaluationVars, - mem: &mut HashMap, F>, + mem: &mut HashMap, F>, ) -> F { match self { ConstraintPolynomialInner::Constant(c) => *c, @@ -445,53 +473,12 @@ impl ConstraintPolynomialInner { base.0.degree() * BigUint::from_usize(*exponent).unwrap(), } } -} -/// Wraps `Rc`, and implements `Hash` and `Eq` based on references rather -/// than content. This is useful when we want to use constraint polynomials as `HashMap` keys, but -/// we want address-based hashing for performance reasons. -#[derive(Clone)] -pub(crate) struct ConstraintPolynomialRef(Rc>); - -impl ConstraintPolynomialRef { - fn new(inner: ConstraintPolynomialInner) -> Self { - Self(Rc::new(inner)) - } - - fn evaluate_memoized( - &self, - vars: &EvaluationVars, - mem: &mut HashMap, - ) -> F { - if let Some(&result) = mem.get(self) { - result - } else { - let result = self.0.evaluate(vars, mem); - mem.insert(self.clone(), result); - result - } - } - - /// Replace all occurrences of `from` with `to` in this polynomial graph. fn replace_all( &self, - from: ConstraintPolynomialRef, - to: ConstraintPolynomialRef, - ) -> ConstraintPolynomialRef { + from: ConstraintPolynomial, + to: ConstraintPolynomial, + ) -> Self { todo!() } } - -impl PartialEq for ConstraintPolynomialRef { - fn eq(&self, other: &Self) -> bool { - ptr::eq(&*self.0, &*other.0) - } -} - -impl Eq for ConstraintPolynomialRef {} - -impl Hash for ConstraintPolynomialRef { - fn hash(&self, state: &mut H) { - ptr::hash(&*self.0, state); - } -} diff --git a/src/gates/output_graph.rs b/src/gates/output_graph.rs index 9815b5df..4bbee83f 100644 --- a/src/gates/output_graph.rs +++ b/src/gates/output_graph.rs @@ -1,6 +1,6 @@ use std::iter; -use crate::constraint_polynomial::{ConstraintPolynomial, ConstraintPolynomialRef}; +use crate::constraint_polynomial::{ConstraintPolynomial}; use crate::field::field::Field; /// Represents a set of deterministic gate outputs, expressed as polynomials over witness