Skip to content

Commit

Permalink
Merge pull request #2131 from ljedrz/perf/smaller_variable
Browse files Browse the repository at this point in the history
Reduce the size of Variable
  • Loading branch information
howardwu authored Nov 15, 2023
2 parents 3afbb2d + e6b0021 commit 7d3ed05
Show file tree
Hide file tree
Showing 6 changed files with 44 additions and 22 deletions.
10 changes: 8 additions & 2 deletions circuit/environment/src/helpers/assignment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,14 @@ impl<F: PrimeField> From<&crate::Variable<F>> for AssignmentVariable<F> {
fn from(variable: &crate::Variable<F>) -> Self {
match variable {
crate::Variable::Constant(value) => Self::Constant(**value),
crate::Variable::Public(index, _) => Self::Public(*index),
crate::Variable::Private(index, _) => Self::Private(*index),
crate::Variable::Public(index_value) => {
let (index, _value) = index_value.as_ref();
Self::Public(*index)
}
crate::Variable::Private(index_value) => {
let (index, _value) = index_value.as_ref();
Self::Private(*index)
}
}
}
}
Expand Down
18 changes: 12 additions & 6 deletions circuit/environment/src/helpers/converter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,15 @@ impl<F: PrimeField> R1CS<F> {
// Allocate the public variables.
for (i, public) in self.to_public_variables().iter().enumerate() {
match public {
Variable::Public(index, value) => {
Variable::Public(index_value) => {
let (index, value) = index_value.as_ref();

assert_eq!(
i as u64, *index,
"Public variables in first system must be processed in lexicographic order"
);

let gadget = cs.alloc_input(|| format!("Public {i}"), || Ok(**value))?;
let gadget = cs.alloc_input(|| format!("Public {i}"), || Ok(*value))?;

assert_eq!(
snarkvm_algorithms::r1cs::Index::Public((index + 1) as usize),
Expand All @@ -75,13 +77,15 @@ impl<F: PrimeField> R1CS<F> {
// Allocate the private variables.
for (i, private) in self.to_private_variables().iter().enumerate() {
match private {
Variable::Private(index, value) => {
Variable::Private(index_value) => {
let (index, value) = index_value.as_ref();

assert_eq!(
i as u64, *index,
"Private variables in first system must be processed in lexicographic order"
);

let gadget = cs.alloc(|| format!("Private {i}"), || Ok(**value))?;
let gadget = cs.alloc(|| format!("Private {i}"), || Ok(*value))?;

assert_eq!(
snarkvm_algorithms::r1cs::Index::Private(i),
Expand Down Expand Up @@ -113,7 +117,8 @@ impl<F: PrimeField> R1CS<F> {
"Failed during constraint translation. The first system by definition cannot have constant variables in the terms"
)
}
Variable::Public(index, _) => {
Variable::Public(index_value) => {
let (index, _value) = index_value.as_ref();
let gadget = converter.public.get(index).unwrap();
assert_eq!(
snarkvm_algorithms::r1cs::Index::Public((index + 1) as usize),
Expand All @@ -122,7 +127,8 @@ impl<F: PrimeField> R1CS<F> {
);
linear_combination += (*coefficient, *gadget);
}
Variable::Private(index, _) => {
Variable::Private(index_value) => {
let (index, _value) = index_value.as_ref();
let gadget = converter.private.get(index).unwrap();
assert_eq!(
snarkvm_algorithms::r1cs::Index::Private(*index as usize),
Expand Down
2 changes: 1 addition & 1 deletion circuit/environment/src/helpers/linear_combination.rs
Original file line number Diff line number Diff line change
Expand Up @@ -549,7 +549,7 @@ mod tests {
let two = one + one;
let four = two + two;

let start = LinearCombination::from(Variable::Public(1, Rc::new(one)));
let start = LinearCombination::from(Variable::Public(Rc::new((1, one))));
assert!(!start.is_constant());
assert_eq!(one, start.value());

Expand Down
6 changes: 3 additions & 3 deletions circuit/environment/src/helpers/r1cs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ impl<F: PrimeField> R1CS<F> {
pub(crate) fn new() -> Self {
Self {
constants: Default::default(),
public: vec![Variable::Public(0u64, Rc::new(F::one()))],
public: vec![Variable::Public(Rc::new((0u64, F::one())))],
private: Default::default(),
constraints: Default::default(),
counter: Default::default(),
Expand Down Expand Up @@ -65,15 +65,15 @@ impl<F: PrimeField> R1CS<F> {

/// Returns a new public variable with the given value and scope.
pub(crate) fn new_public(&mut self, value: F) -> Variable<F> {
let variable = Variable::Public(self.public.len() as u64, Rc::new(value));
let variable = Variable::Public(Rc::new((self.public.len() as u64, value)));
self.public.push(variable.clone());
self.counter.increment_public();
variable
}

/// Returns a new private variable with the given value and scope.
pub(crate) fn new_private(&mut self, value: F) -> Variable<F> {
let variable = Variable::Private(self.private.len() as u64, Rc::new(value));
let variable = Variable::Private(Rc::new((self.private.len() as u64, value)));
self.private.push(variable.clone());
self.counter.increment_private();
variable
Expand Down
28 changes: 19 additions & 9 deletions circuit/environment/src/helpers/variable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ pub type Index = u64;
#[derive(Clone, PartialEq, Eq, Hash)]
pub enum Variable<F: PrimeField> {
Constant(Rc<F>),
Public(Index, Rc<F>),
Private(Index, Rc<F>),
Public(Rc<(Index, F)>),
Private(Rc<(Index, F)>),
}

impl<F: PrimeField> Variable<F> {
Expand Down Expand Up @@ -70,8 +70,10 @@ impl<F: PrimeField> Variable<F> {
pub fn index(&self) -> Index {
match self {
Self::Constant(..) => 0,
Self::Public(index, ..) => *index,
Self::Private(index, ..) => *index,
Self::Public(index_value) | Self::Private(index_value) => {
let (index, _value) = index_value.as_ref();
*index
}
}
}

Expand All @@ -81,8 +83,10 @@ impl<F: PrimeField> Variable<F> {
pub fn value(&self) -> F {
match self {
Self::Constant(value) => **value,
Self::Public(_, value) => **value,
Self::Private(_, value) => **value,
Self::Public(index_value) | Self::Private(index_value) => {
let (_index, value) = index_value.as_ref();
*value
}
}
}
}
Expand Down Expand Up @@ -259,8 +263,14 @@ impl<F: PrimeField> fmt::Debug for Variable<F> {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
write!(f, "{}", match self {
Self::Constant(value) => format!("Constant({value})"),
Self::Public(index, value) => format!("Public({index}, {value})"),
Self::Private(index, value) => format!("Private({index}, {value})"),
Self::Public(index_value) => {
let (index, value) = index_value.as_ref();
format!("Public({index}, {value})")
}
Self::Private(index_value) => {
let (index, value) = index_value.as_ref();
format!("Private({index}, {value})")
}
})
}
}
Expand All @@ -277,6 +287,6 @@ mod tests {

#[test]
fn test_size() {
assert_eq!(24, std::mem::size_of::<Variable<<Circuit as Environment>::BaseField>>());
assert_eq!(16, std::mem::size_of::<Variable<<Circuit as Environment>::BaseField>>());
}
}
2 changes: 1 addition & 1 deletion circuit/types/boolean/src/not.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ impl<E: Environment> Not for &Boolean<E> {
// Public and private cases.
// Note: We directly instantiate a public variable to correctly represent a boolean in a linear combination.
// For more information, see `LinearCombination::is_boolean_type`.
false => Boolean(Variable::Public(0, Rc::new(E::BaseField::one())) - &self.0),
false => Boolean(Variable::Public(Rc::new((0, E::BaseField::one()))) - &self.0),
}
}
}
Expand Down

0 comments on commit 7d3ed05

Please sign in to comment.