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.

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

feat(profiler): Add Brillig procedure info to debug artifact for more informative profiling #6385

Merged
merged 11 commits into from
Oct 30, 2024
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 11 additions & 1 deletion compiler/noirc_errors/src/debug_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@ pub type DebugVariables = BTreeMap<DebugVarId, DebugVariable>;
pub type DebugFunctions = BTreeMap<DebugFnId, DebugFunction>;
pub type DebugTypes = BTreeMap<DebugTypeId, PrintableType>;

#[derive(Debug, Clone, Copy, Hash, PartialEq, Eq, PartialOrd, Ord, Deserialize, Serialize)]
pub struct ProcedureDebugId(pub u32);

#[derive(Default, Debug, Clone, Deserialize, Serialize)]
pub struct ProgramDebugInfo {
pub debug_infos: Vec<DebugInfo>,
Expand Down Expand Up @@ -104,6 +107,9 @@ pub struct DebugInfo {
pub variables: DebugVariables,
pub functions: DebugFunctions,
pub types: DebugTypes,
/// This a map per brillig function representing the range of opcodes where a procedure is activated.
pub brillig_procedure_locs:
BTreeMap<BrilligFunctionId, BTreeMap<ProcedureDebugId, (usize, usize)>>,
}

/// Holds OpCodes Counts for Acir and Brillig Opcodes
Expand All @@ -124,8 +130,12 @@ impl DebugInfo {
variables: DebugVariables,
functions: DebugFunctions,
types: DebugTypes,
brillig_procedure_locs: BTreeMap<
BrilligFunctionId,
BTreeMap<ProcedureDebugId, (usize, usize)>,
>,
) -> Self {
Self { locations, brillig_locations, variables, functions, types }
Self { locations, brillig_locations, variables, functions, types, brillig_procedure_locs }
}

/// Updates the locations map when the [`Circuit`][acvm::acir::circuit::Circuit] is modified.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,8 @@ pub(crate) fn directive_invert<F: AcirField>() -> GeneratedBrillig<F> {
},
BrilligOpcode::Stop { return_data_offset: 0, return_data_size: 1 },
],
assert_messages: Default::default(),
locations: Default::default(),
name: "directive_invert".to_string(),
..Default::default()
}
}

Expand Down Expand Up @@ -132,8 +131,7 @@ pub(crate) fn directive_quotient<F: AcirField>() -> GeneratedBrillig<F> {
},
BrilligOpcode::Stop { return_data_offset: 0, return_data_size: 2 },
],
assert_messages: Default::default(),
locations: Default::default(),
name: "directive_integer_quotient".to_string(),
..Default::default()
}
}
13 changes: 12 additions & 1 deletion compiler/noirc_evaluator/src/brillig/brillig_ir/artifact.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
use acvm::acir::brillig::Opcode as BrilligOpcode;
use std::collections::{BTreeMap, HashMap};

use crate::brillig::brillig_ir::procedures::ProcedureId;
use crate::ssa::ir::{basic_block::BasicBlockId, dfg::CallStack, function::FunctionId};

use super::procedures::ProcedureId;

/// Represents a parameter or a return value of an entry point function.
#[derive(Debug, Clone, Eq, PartialEq, Hash, PartialOrd, Ord)]
pub(crate) enum BrilligParameter {
Expand All @@ -24,6 +25,7 @@ pub(crate) struct GeneratedBrillig<F> {
pub(crate) locations: BTreeMap<OpcodeLocation, CallStack>,
pub(crate) assert_messages: BTreeMap<OpcodeLocation, String>,
pub(crate) name: String,
pub(crate) procedure_locations: HashMap<ProcedureId, (OpcodeLocation, OpcodeLocation)>,
}

#[derive(Default, Debug, Clone)]
Expand Down Expand Up @@ -53,6 +55,14 @@ pub(crate) struct BrilligArtifact<F> {
call_stack: CallStack,
/// Name of the function, only used for debugging purposes.
pub(crate) name: String,

/// This field contains the given procedure id if this artifact originates from as procedure
pub(crate) procedure: Option<ProcedureId>,
/// Procedure ID mapped to the range of its opcode locations
/// This is created as artifacts are linked together and allows us to determine
/// which opcodes originate from reusable procedures.s
/// The range is inclusive for both start and end opcode locations.
pub(crate) procedure_locations: HashMap<ProcedureId, (OpcodeLocation, OpcodeLocation)>,
}

/// A pointer to a location in the opcode.
Expand Down Expand Up @@ -149,6 +159,7 @@ impl<F: Clone + std::fmt::Debug> BrilligArtifact<F> {
locations: self.locations,
assert_messages: self.assert_messages,
name: self.name,
procedure_locations: self.procedure_locations,
}
}

Expand Down
64 changes: 62 additions & 2 deletions compiler/noirc_evaluator/src/brillig/brillig_ir/procedures/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,10 @@ use array_copy::compile_array_copy_procedure;
use array_reverse::compile_array_reverse_procedure;
use check_max_stack_depth::compile_check_max_stack_depth_procedure;
use mem_copy::compile_mem_copy_procedure;
use noirc_errors::debug_info::ProcedureDebugId;
use prepare_vector_insert::compile_prepare_vector_insert_procedure;
use prepare_vector_push::compile_prepare_vector_push_procedure;
use serde::{Deserialize, Serialize};
use vector_copy::compile_vector_copy_procedure;
use vector_pop_back::compile_vector_pop_back_procedure;
use vector_pop_front::compile_vector_pop_front_procedure;
Expand All @@ -31,8 +33,8 @@ use super::{
/// Procedures are a set of complex operations that are common in the noir language.
/// Extracting them to reusable procedures allows us to reduce the size of the generated Brillig.
/// Procedures receive their arguments on scratch space to avoid stack dumping&restoring.
#[derive(Debug, Clone, Copy, Eq, PartialEq, Hash)]
pub(crate) enum ProcedureId {
#[derive(Debug, Clone, Copy, Eq, PartialEq, Hash, PartialOrd, Ord, Deserialize, Serialize)]
pub enum ProcedureId {
ArrayCopy,
ArrayReverse,
VectorCopy,
Expand All @@ -45,6 +47,64 @@ pub(crate) enum ProcedureId {
CheckMaxStackDepth,
}

impl ProcedureId {
pub(crate) fn to_debug_id(self) -> ProcedureDebugId {
ProcedureDebugId(match self {
ProcedureId::ArrayCopy => 0,
ProcedureId::ArrayReverse => 1,
ProcedureId::VectorCopy => 2,
ProcedureId::MemCopy => 3,
ProcedureId::PrepareVectorPush(flag) => {
if flag {
4
} else {
5
}
}
vezenovm marked this conversation as resolved.
Show resolved Hide resolved
ProcedureId::VectorPopFront => 6,
ProcedureId::VectorPopBack => 7,
ProcedureId::PrepareVectorInsert => 8,
ProcedureId::VectorRemove => 9,
ProcedureId::CheckMaxStackDepth => 10,
})
}

pub fn from_debug_id(debug_id: ProcedureDebugId) -> Self {
let inner = debug_id.0;
match inner {
0 => ProcedureId::ArrayCopy,
1 => ProcedureId::ArrayReverse,
2 => ProcedureId::VectorCopy,
3 => ProcedureId::MemCopy,
4 => ProcedureId::PrepareVectorPush(true),
5 => ProcedureId::PrepareVectorPush(false),
6 => ProcedureId::VectorPopFront,
7 => ProcedureId::VectorPopBack,
8 => ProcedureId::PrepareVectorInsert,
9 => ProcedureId::VectorRemove,
10 => ProcedureId::CheckMaxStackDepth,
_ => panic!("Unsupported procedure debug ID of {inner} was supplied"),
}
}
}

impl std::fmt::Display for ProcedureId {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
ProcedureId::ArrayCopy => write!(f, "ArrayCopy"),
ProcedureId::ArrayReverse => write!(f, "ArrayReverse"),
ProcedureId::VectorCopy => write!(f, "VectorCopy"),
ProcedureId::MemCopy => write!(f, "MemCopy"),
ProcedureId::PrepareVectorPush(flag) => write!(f, "PrepareVectorPush({flag})"),
ProcedureId::VectorPopFront => write!(f, "VectorPopFront"),
ProcedureId::VectorPopBack => write!(f, "VectorPopBack"),
ProcedureId::PrepareVectorInsert => write!(f, "PrepareVectorInsert"),
ProcedureId::VectorRemove => write!(f, "VectorRemove"),
ProcedureId::CheckMaxStackDepth => write!(f, "CheckMaxStackDepth"),
}
}
}

pub(crate) fn compile_procedure<F: AcirField + DebugToString>(
procedure_id: ProcedureId,
) -> BrilligArtifact<F> {
Expand Down
8 changes: 7 additions & 1 deletion compiler/noirc_evaluator/src/brillig/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ use crate::ssa::{
use fxhash::FxHashMap as HashMap;
use std::{borrow::Cow, collections::BTreeSet};

pub use self::brillig_ir::procedures::ProcedureId;

/// Context structure for the brillig pass.
/// It stores brillig-related data required for brillig generation.
#[derive(Default)]
Expand All @@ -43,7 +45,11 @@ impl Brillig {
self.ssa_function_to_brillig.get(&function_id).map(Cow::Borrowed)
}
// Procedures are compiled as needed
LabelType::Procedure(procedure_id) => Some(Cow::Owned(compile_procedure(procedure_id))),
LabelType::Procedure(procedure_id) => {
let mut artifact = compile_procedure(procedure_id);
artifact.procedure = Some(procedure_id);
Some(Cow::Owned(artifact))
vezenovm marked this conversation as resolved.
Show resolved Hide resolved
}
_ => unreachable!("ICE: Expected a function or procedure label"),
}
}
Expand Down
11 changes: 9 additions & 2 deletions compiler/noirc_evaluator/src/ssa.rs
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,7 @@ fn convert_generated_acir_into_circuit(
assertion_payloads: assert_messages,
warnings,
name,
brillig_procedure_locs,
..
} = generated_acir;

Expand Down Expand Up @@ -328,8 +329,14 @@ fn convert_generated_acir_into_circuit(
})
.collect();

let mut debug_info =
DebugInfo::new(locations, brillig_locations, debug_variables, debug_functions, debug_types);
let mut debug_info = DebugInfo::new(
locations,
brillig_locations,
debug_variables,
debug_functions,
debug_types,
brillig_procedure_locs,
);

// Perform any ACIR-level optimizations
let (optimized_circuit, transformation_map) = acvm::compiler::optimize(circuit);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@ use acvm::{
acir::AcirField,
acir::{circuit::directives::Directive, native_types::Expression},
};

use iter_extended::vecmap;
use noirc_errors::debug_info::ProcedureDebugId;
use num_bigint::BigUint;

/// Brillig calls such as for the Brillig std lib are resolved only after code generation is finished.
Expand Down Expand Up @@ -72,13 +74,20 @@ pub(crate) struct GeneratedAcir<F: AcirField> {
/// As to avoid passing the ACIR gen shared context into each individual ACIR
/// we can instead keep this map and resolve the Brillig calls at the end of code generation.
pub(crate) brillig_stdlib_func_locations: BTreeMap<OpcodeLocation, BrilligStdlibFunc>,

/// Brillig function id -> Brillig procedure locations map
/// This maps allows a profiler to determine which Brillig opcodes
/// originated from a reusable procedure.
pub(crate) brillig_procedure_locs: BTreeMap<BrilligFunctionId, BrilligProcedureRangeMap>,
}

/// Correspondence between an opcode index (in opcodes) and the source code call stack which generated it
pub(crate) type OpcodeToLocationsMap = BTreeMap<OpcodeLocation, CallStack>;

pub(crate) type BrilligOpcodeToLocationsMap = BTreeMap<BrilligOpcodeLocation, CallStack>;

pub(crate) type BrilligProcedureRangeMap = BTreeMap<ProcedureDebugId, (usize, usize)>;

#[derive(Debug, Clone, Copy, Hash, PartialEq, Eq)]
pub(crate) enum BrilligStdlibFunc {
Inverse,
Expand Down Expand Up @@ -591,6 +600,14 @@ impl<F: AcirField> GeneratedAcir<F> {
return;
}

for (procedure_id, (start_index, end_index)) in generated_brillig.procedure_locations.iter()
{
self.brillig_procedure_locs
.entry(brillig_function_index)
.or_default()
.insert(procedure_id.to_debug_id(), (*start_index, *end_index));
}

for (brillig_index, call_stack) in generated_brillig.locations.iter() {
self.brillig_locations
.entry(brillig_function_index)
Expand Down
9 changes: 9 additions & 0 deletions compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1003,6 +1003,15 @@ impl<'a> Context<'a> {
}
};
entry_point.link_with(artifact);
// Insert the range of opcode locations occupied by a procedure
if let Some(procedure_id) = artifact.procedure {
let num_opcodes = entry_point.byte_code.len();
let previous_num_opcodes = entry_point.byte_code.len() - artifact.byte_code.len();
// We subtract one as to keep the range inclusive on both ends
entry_point
.procedure_locations
.insert(procedure_id, (previous_num_opcodes, num_opcodes - 1));
}
}
// Generate the final bytecode
Ok(entry_point.finish())
Expand Down
1 change: 1 addition & 0 deletions tooling/debugger/src/source_code_printer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,7 @@ mod tests {
BTreeMap::default(),
BTreeMap::default(),
BTreeMap::default(),
BTreeMap::default(),
)];
let debug_artifact = DebugArtifact::new(debug_symbols, &fm);

Expand Down
1 change: 0 additions & 1 deletion tooling/nargo_cli/src/cli/compile_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,6 @@ fn compile_programs(
let target_width =
get_target_width(package.expression_width, compile_options.expression_width);
let program = nargo::ops::transform_program(program, target_width);

save_program_to_file(&program.into(), &package.name, workspace.target_directory_path());

Ok(((), warnings))
Expand Down
1 change: 1 addition & 0 deletions tooling/noirc_artifacts/src/debug.rs
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,7 @@ mod tests {
BTreeMap::default(),
BTreeMap::default(),
BTreeMap::default(),
BTreeMap::default(),
)];
let debug_artifact = DebugArtifact::new(debug_symbols, &fm);

Expand Down
1 change: 1 addition & 0 deletions tooling/profiler/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ acir.workspace = true
nargo.workspace = true
noirc_errors.workspace = true
noirc_abi.workspace = true
noirc_evaluator.workspace = true

# Logs
tracing-subscriber.workspace = true
Expand Down
21 changes: 20 additions & 1 deletion tooling/profiler/src/flamegraph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use inferno::flamegraph::{from_lines, Options, TextTruncateDirection};
use noirc_errors::debug_info::DebugInfo;
use noirc_errors::reporter::line_and_column_from_span;
use noirc_errors::Location;
use noirc_evaluator::brillig::ProcedureId;

use crate::opcode_formatter::AcirOrBrilligOpcode;

Expand Down Expand Up @@ -111,6 +112,7 @@ fn generate_folded_sorted_lines<'files, F: AcirField>(
if let Some(opcode) = &sample.opcode {
location_names.push(format_opcode(opcode));
}

add_locations_to_folded_stack_items(&mut folded_stack_items, location_names, sample.count);
}

Expand All @@ -123,10 +125,20 @@ fn find_callsite_labels<'files>(
brillig_function_id: Option<BrilligFunctionId>,
files: &'files impl Files<'files, FileId = fm::FileId>,
) -> Vec<String> {
let mut procedure_id = None;
let source_locations = debug_symbols.opcode_location(opcode_location).unwrap_or_else(|| {
if let (Some(brillig_function_id), Some(brillig_location)) =
(brillig_function_id, opcode_location.to_brillig_location())
{
let procedure_locs = debug_symbols.brillig_procedure_locs.get(&brillig_function_id);
if let Some(procedure_locs) = procedure_locs {
for (procedure, range) in procedure_locs.iter() {
if brillig_location.0 >= range.0 && brillig_location.0 <= range.1 {
procedure_id = Some(*procedure);
break;
}
}
}
let brillig_locations = debug_symbols.brillig_locations.get(&brillig_function_id);
if let Some(brillig_locations) = brillig_locations {
brillig_locations.get(&brillig_location).cloned().unwrap_or_default()
Expand All @@ -137,10 +149,16 @@ fn find_callsite_labels<'files>(
vec![]
}
});
let callsite_labels: Vec<_> = source_locations

let mut callsite_labels: Vec<_> = source_locations
.into_iter()
.map(|location| location_to_callsite_label(location, files))
.collect();

if let Some(procedure_id) = procedure_id {
callsite_labels.push(format!("procedure::{}", ProcedureId::from_debug_id(procedure_id)));
}

callsite_labels
}

Expand Down Expand Up @@ -317,6 +335,7 @@ mod tests {
BTreeMap::default(),
BTreeMap::default(),
BTreeMap::default(),
BTreeMap::default(),
);

let samples: Vec<Sample<FieldElement>> = vec![
Expand Down
Loading