From 4995881b9f67af9e3d3d2d80e37bde7ba7d9d8e1 Mon Sep 17 00:00:00 2001 From: sirasistant Date: Fri, 15 Dec 2023 10:03:33 +0000 Subject: [PATCH 1/2] fix: debug info performance --- acvm-repo/acvm/src/compiler/mod.rs | 37 +++++++++++++------ acvm-repo/acvm/src/compiler/optimizers/mod.rs | 10 ++--- .../acvm/src/compiler/transformers/mod.rs | 14 +++---- 3 files changed, 36 insertions(+), 25 deletions(-) diff --git a/acvm-repo/acvm/src/compiler/mod.rs b/acvm-repo/acvm/src/compiler/mod.rs index 4abf94a2e7..5d5795d4bc 100644 --- a/acvm-repo/acvm/src/compiler/mod.rs +++ b/acvm-repo/acvm/src/compiler/mod.rs @@ -1,3 +1,5 @@ +use std::collections::HashMap; + use acir::{ circuit::{opcodes::UnsupportedMemoryOpcode, Circuit, Opcode, OpcodeLocation}, BlackBoxFunc, @@ -27,12 +29,22 @@ pub enum CompileError { /// metadata they had about the opcodes to the new opcode structure generated after the transformation. #[derive(Debug)] pub struct AcirTransformationMap { - /// This is a vector of pointers to the old acir opcodes. The index of the vector is the new opcode index. - /// The value of the vector is the old opcode index pointed. - acir_opcode_positions: Vec, + /// Maps the old acir indexes to the new acir indexes + old_indexes_to_new_indexes: HashMap>, } impl AcirTransformationMap { + /// Builds a map from a vector of pointers to the old acir opcodes. + /// The index of the vector is the new opcode index. + /// The value of the vector is the old opcode index pointed. + fn new(acir_opcode_positions: Vec) -> Self { + let mut old_indexes_to_new_indexes = HashMap::with_capacity(acir_opcode_positions.len()); + for (new_index, old_index) in acir_opcode_positions.into_iter().enumerate() { + old_indexes_to_new_indexes.entry(old_index).or_insert_with(Vec::new).push(new_index); + } + AcirTransformationMap { old_indexes_to_new_indexes } + } + pub fn new_locations( &self, old_location: OpcodeLocation, @@ -41,17 +53,16 @@ impl AcirTransformationMap { OpcodeLocation::Acir(index) => index, OpcodeLocation::Brillig { acir_index, .. } => acir_index, }; + let new_indexes = self.old_indexes_to_new_indexes.get(&old_acir_index); - self.acir_opcode_positions - .iter() - .enumerate() - .filter(move |(_, &old_index)| old_index == old_acir_index) - .map(move |(new_index, _)| match old_location { - OpcodeLocation::Acir(_) => OpcodeLocation::Acir(new_index), + new_indexes.into_iter().flat_map(move |new_indexes| { + new_indexes.iter().map(move |new_index| match old_location { + OpcodeLocation::Acir(_) => OpcodeLocation::Acir(*new_index), OpcodeLocation::Brillig { brillig_index, .. } => { - OpcodeLocation::Brillig { acir_index: new_index, brillig_index } + OpcodeLocation::Brillig { acir_index: *new_index, brillig_index } } }) + }) } } @@ -74,11 +85,13 @@ pub fn compile( np_language: Language, is_opcode_supported: impl Fn(&Opcode) -> bool, ) -> Result<(Circuit, AcirTransformationMap), CompileError> { - let (acir, AcirTransformationMap { acir_opcode_positions }) = optimize_internal(acir); + let (acir, acir_opcode_positions) = optimize_internal(acir); - let (mut acir, transformation_map) = + let (mut acir, acir_opcode_positions) = transform_internal(acir, np_language, is_opcode_supported, acir_opcode_positions)?; + let transformation_map = AcirTransformationMap::new(acir_opcode_positions); + acir.assert_messages = transform_assert_messages(acir.assert_messages, &transformation_map); Ok((acir, transformation_map)) diff --git a/acvm-repo/acvm/src/compiler/optimizers/mod.rs b/acvm-repo/acvm/src/compiler/optimizers/mod.rs index 6e2af4f58a..85a97c2c7d 100644 --- a/acvm-repo/acvm/src/compiler/optimizers/mod.rs +++ b/acvm-repo/acvm/src/compiler/optimizers/mod.rs @@ -13,7 +13,9 @@ use super::{transform_assert_messages, AcirTransformationMap}; /// Applies [`ProofSystemCompiler`][crate::ProofSystemCompiler] independent optimizations to a [`Circuit`]. pub fn optimize(acir: Circuit) -> (Circuit, AcirTransformationMap) { - let (mut acir, transformation_map) = optimize_internal(acir); + let (mut acir, new_opcode_positions) = optimize_internal(acir); + + let transformation_map = AcirTransformationMap::new(new_opcode_positions); acir.assert_messages = transform_assert_messages(acir.assert_messages, &transformation_map); @@ -21,7 +23,7 @@ pub fn optimize(acir: Circuit) -> (Circuit, AcirTransformationMap) { } /// Applies [`ProofSystemCompiler`][crate::ProofSystemCompiler] independent optimizations to a [`Circuit`]. -pub(super) fn optimize_internal(acir: Circuit) -> (Circuit, AcirTransformationMap) { +pub(super) fn optimize_internal(acir: Circuit) -> (Circuit, Vec) { log::trace!("Start circuit optimization"); // General optimizer pass @@ -52,9 +54,7 @@ pub(super) fn optimize_internal(acir: Circuit) -> (Circuit, AcirTransformationMa let (acir, acir_opcode_positions) = range_optimizer.replace_redundant_ranges(acir_opcode_positions); - let transformation_map = AcirTransformationMap { acir_opcode_positions }; - log::trace!("Finish circuit optimization"); - (acir, transformation_map) + (acir, acir_opcode_positions) } diff --git a/acvm-repo/acvm/src/compiler/transformers/mod.rs b/acvm-repo/acvm/src/compiler/transformers/mod.rs index 00ee9dc7ce..c4c94e371b 100644 --- a/acvm-repo/acvm/src/compiler/transformers/mod.rs +++ b/acvm-repo/acvm/src/compiler/transformers/mod.rs @@ -27,9 +27,11 @@ pub fn transform( // by applying the modifications done to the circuit opcodes and also to the opcode_positions (delete and insert) let acir_opcode_positions = acir.opcodes.iter().enumerate().map(|(i, _)| i).collect(); - let (mut acir, transformation_map) = + let (mut acir, acir_opcode_positions) = transform_internal(acir, np_language, is_opcode_supported, acir_opcode_positions)?; + let transformation_map = AcirTransformationMap::new(acir_opcode_positions); + acir.assert_messages = transform_assert_messages(acir.assert_messages, &transformation_map); Ok((acir, transformation_map)) @@ -43,7 +45,7 @@ pub(super) fn transform_internal( np_language: Language, is_opcode_supported: impl Fn(&Opcode) -> bool, acir_opcode_positions: Vec, -) -> Result<(Circuit, AcirTransformationMap), CompileError> { +) -> Result<(Circuit, Vec), CompileError> { log::trace!("Start circuit transformation"); // Fallback transformer pass @@ -52,9 +54,8 @@ pub(super) fn transform_internal( let mut transformer = match &np_language { crate::Language::R1CS => { - let transformation_map = AcirTransformationMap { acir_opcode_positions }; let transformer = R1CSTransformer::new(acir); - return Ok((transformer.transform(), transformation_map)); + return Ok((transformer.transform(), acir_opcode_positions)); } crate::Language::PLONKCSat { width } => { let mut csat = CSatTransformer::new(*width); @@ -216,10 +217,7 @@ pub(super) fn transform_internal( ..acir }; - let transformation_map = - AcirTransformationMap { acir_opcode_positions: new_acir_opcode_positions }; - log::trace!("Finish circuit transformation"); - Ok((acir, transformation_map)) + Ok((acir, new_acir_opcode_positions)) } From 0b7de0fc31367a753bb871c0e0b626b458135cd9 Mon Sep 17 00:00:00 2001 From: sirasistant Date: Fri, 15 Dec 2023 10:11:06 +0000 Subject: [PATCH 2/2] refactor: update naming after PR comments --- acvm-repo/acvm/src/compiler/mod.rs | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/acvm-repo/acvm/src/compiler/mod.rs b/acvm-repo/acvm/src/compiler/mod.rs index 5d5795d4bc..0ed5b59167 100644 --- a/acvm-repo/acvm/src/compiler/mod.rs +++ b/acvm-repo/acvm/src/compiler/mod.rs @@ -29,8 +29,8 @@ pub enum CompileError { /// metadata they had about the opcodes to the new opcode structure generated after the transformation. #[derive(Debug)] pub struct AcirTransformationMap { - /// Maps the old acir indexes to the new acir indexes - old_indexes_to_new_indexes: HashMap>, + /// Maps the old acir indices to the new acir indices + old_indices_to_new_indices: HashMap>, } impl AcirTransformationMap { @@ -38,11 +38,11 @@ impl AcirTransformationMap { /// The index of the vector is the new opcode index. /// The value of the vector is the old opcode index pointed. fn new(acir_opcode_positions: Vec) -> Self { - let mut old_indexes_to_new_indexes = HashMap::with_capacity(acir_opcode_positions.len()); + let mut old_indices_to_new_indices = HashMap::with_capacity(acir_opcode_positions.len()); for (new_index, old_index) in acir_opcode_positions.into_iter().enumerate() { - old_indexes_to_new_indexes.entry(old_index).or_insert_with(Vec::new).push(new_index); + old_indices_to_new_indices.entry(old_index).or_insert_with(Vec::new).push(new_index); } - AcirTransformationMap { old_indexes_to_new_indexes } + AcirTransformationMap { old_indices_to_new_indices } } pub fn new_locations( @@ -53,16 +53,17 @@ impl AcirTransformationMap { OpcodeLocation::Acir(index) => index, OpcodeLocation::Brillig { acir_index, .. } => acir_index, }; - let new_indexes = self.old_indexes_to_new_indexes.get(&old_acir_index); - new_indexes.into_iter().flat_map(move |new_indexes| { - new_indexes.iter().map(move |new_index| match old_location { - OpcodeLocation::Acir(_) => OpcodeLocation::Acir(*new_index), - OpcodeLocation::Brillig { brillig_index, .. } => { - OpcodeLocation::Brillig { acir_index: *new_index, brillig_index } - } - }) - }) + self.old_indices_to_new_indices.get(&old_acir_index).into_iter().flat_map( + move |new_indices| { + new_indices.iter().map(move |new_index| match old_location { + OpcodeLocation::Acir(_) => OpcodeLocation::Acir(*new_index), + OpcodeLocation::Brillig { brillig_index, .. } => { + OpcodeLocation::Brillig { acir_index: *new_index, brillig_index } + } + }) + }, + ) } }