From 8af441ea7a461d5755be1e0c476b296b7f6fcb6e Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Mon, 23 Oct 2023 16:19:01 +0100 Subject: [PATCH] feat: Cache debug artifacts (#3133) Co-authored-by: kevaundray --- .../nargo_cli/src/cli/codegen_verifier_cmd.rs | 1 - tooling/nargo_cli/src/cli/compile_cmd.rs | 98 ++++++------------- tooling/nargo_cli/src/cli/debug_cmd.rs | 12 +-- tooling/nargo_cli/src/cli/execute_cmd.rs | 1 - tooling/nargo_cli/src/cli/fs/program.rs | 11 +++ tooling/nargo_cli/src/cli/info_cmd.rs | 1 - tooling/nargo_cli/src/cli/prove_cmd.rs | 1 - tooling/nargo_cli/src/cli/verify_cmd.rs | 1 - 8 files changed, 43 insertions(+), 83 deletions(-) diff --git a/tooling/nargo_cli/src/cli/codegen_verifier_cmd.rs b/tooling/nargo_cli/src/cli/codegen_verifier_cmd.rs index 35538ef1a83..856970544b8 100644 --- a/tooling/nargo_cli/src/cli/codegen_verifier_cmd.rs +++ b/tooling/nargo_cli/src/cli/codegen_verifier_cmd.rs @@ -76,7 +76,6 @@ fn smart_contract_for_package( workspace, package, compile_options, - false, np_language, &is_opcode_supported, )?; diff --git a/tooling/nargo_cli/src/cli/compile_cmd.rs b/tooling/nargo_cli/src/cli/compile_cmd.rs index a332d63e062..e3161b6d13d 100644 --- a/tooling/nargo_cli/src/cli/compile_cmd.rs +++ b/tooling/nargo_cli/src/cli/compile_cmd.rs @@ -1,4 +1,3 @@ -use std::collections::BTreeMap; use std::path::Path; use acvm::acir::circuit::Opcode; @@ -15,7 +14,6 @@ use nargo::prepare_package; use nargo::workspace::Workspace; use nargo_toml::{get_package_manifest, resolve_workspace_from_toml, PackageSelection}; use noirc_driver::{CompilationResult, CompileOptions, CompiledContract, CompiledProgram}; -use noirc_errors::debug_info::DebugInfo; use noirc_frontend::graph::CrateName; use clap::Args; @@ -23,9 +21,9 @@ use clap::Args; use crate::backends::Backend; use crate::errors::{CliError, CompileError}; -use super::fs::program::read_program_from_file; use super::fs::program::{ - save_contract_to_file, save_debug_artifact_to_file, save_program_to_file, + read_debug_artifact_from_file, read_program_from_file, save_contract_to_file, + save_debug_artifact_to_file, save_program_to_file, }; use super::NargoConfig; use rayon::prelude::*; @@ -40,10 +38,6 @@ pub(crate) struct CompileCommand { #[arg(long)] include_keys: bool, - /// Output debug files - #[arg(long, hide = true)] - output_debug: bool, - /// The name of the package to compile #[clap(long, conflicts_with = "workspace")] package: Option, @@ -82,12 +76,11 @@ pub(crate) fn run( np_language, &opcode_support, &args.compile_options, - args.output_debug, )?; // Save build artifacts to disk. for (package, contract) in contract_packages.into_iter().zip(compiled_contracts) { - save_contract(contract, &package, &circuit_dir, args.output_debug); + save_contract(contract, &package, &circuit_dir); } Ok(()) @@ -100,7 +93,6 @@ pub(super) fn compile_workspace( np_language: Language, opcode_support: &BackendOpcodeSupport, compile_options: &CompileOptions, - output_debug: bool, ) -> Result<(Vec, Vec), CliError> { let is_opcode_supported = |opcode: &_| opcode_support.is_opcode_supported(opcode); @@ -108,14 +100,7 @@ pub(super) fn compile_workspace( let program_results: Vec<(FileManager, CompilationResult)> = binary_packages .par_iter() .map(|package| { - compile_program( - workspace, - package, - compile_options, - output_debug, - np_language, - &is_opcode_supported, - ) + compile_program(workspace, package, compile_options, np_language, &is_opcode_supported) }) .collect(); let contract_results: Vec<(FileManager, CompilationResult)> = @@ -157,7 +142,6 @@ pub(crate) fn compile_bin_package( workspace: &Workspace, package: &Package, compile_options: &CompileOptions, - output_debug: bool, np_language: Language, is_opcode_supported: &impl Fn(&Opcode) -> bool, ) -> Result { @@ -165,14 +149,8 @@ pub(crate) fn compile_bin_package( return Err(CompileError::LibraryCrate(package.name.clone()).into()); } - let (file_manager, compilation_result) = compile_program( - workspace, - package, - compile_options, - output_debug, - np_language, - &is_opcode_supported, - ); + let (file_manager, compilation_result) = + compile_program(workspace, package, compile_options, np_language, &is_opcode_supported); let program = report_errors( compilation_result, @@ -188,37 +166,36 @@ fn compile_program( workspace: &Workspace, package: &Package, compile_options: &CompileOptions, - output_debug: bool, np_language: Language, is_opcode_supported: &impl Fn(&Opcode) -> bool, ) -> (FileManager, CompilationResult) { let (mut context, crate_id) = prepare_package(package, Box::new(|path| std::fs::read_to_string(path))); - let cached_program = if let Ok(preprocessed_program) = - read_program_from_file(workspace.package_build_path(package)) - { - // TODO: Load debug information. + let program_artifact_path = workspace.package_build_path(package); + let mut debug_artifact_path = program_artifact_path.clone(); + debug_artifact_path.set_file_name(format!("debug_{}.json", package.name)); + let cached_program = if let (Ok(preprocessed_program), Ok(mut debug_artifact)) = ( + read_program_from_file(program_artifact_path), + read_debug_artifact_from_file(debug_artifact_path), + ) { Some(CompiledProgram { hash: preprocessed_program.hash, circuit: preprocessed_program.bytecode, abi: preprocessed_program.abi, - debug: DebugInfo::default(), - file_map: BTreeMap::new(), + debug: debug_artifact.debug_symbols.remove(0), + file_map: debug_artifact.file_map, }) } else { None }; - // If we want to output the debug information then we need to perform a full recompilation of the ACIR. - let force_recompile = output_debug; - let (program, warnings) = match noirc_driver::compile_main( &mut context, crate_id, compile_options, cached_program, - force_recompile, + false, ) { Ok(program_and_warnings) => program_and_warnings, Err(errors) => { @@ -231,12 +208,7 @@ fn compile_program( nargo::ops::optimize_program(program, np_language, &is_opcode_supported) .expect("Backend does not support an opcode that is in the IR"); - save_program( - optimized_program.clone(), - package, - &workspace.target_directory_path(), - output_debug, - ); + save_program(optimized_program.clone(), package, &workspace.target_directory_path()); (context.file_manager, Ok((optimized_program, warnings))) } @@ -264,12 +236,7 @@ fn compile_contract( (context.file_manager, Ok((optimized_contract, warnings))) } -fn save_program( - program: CompiledProgram, - package: &Package, - circuit_dir: &Path, - output_debug: bool, -) { +fn save_program(program: CompiledProgram, package: &Package, circuit_dir: &Path) { let preprocessed_program = PreprocessedProgram { hash: program.hash, backend: String::from(BACKEND_IDENTIFIER), @@ -279,20 +246,13 @@ fn save_program( save_program_to_file(&preprocessed_program, &package.name, circuit_dir); - if output_debug { - let debug_artifact = - DebugArtifact { debug_symbols: vec![program.debug], file_map: program.file_map }; - let circuit_name: String = (&package.name).into(); - save_debug_artifact_to_file(&debug_artifact, &circuit_name, circuit_dir); - } + let debug_artifact = + DebugArtifact { debug_symbols: vec![program.debug], file_map: program.file_map }; + let circuit_name: String = (&package.name).into(); + save_debug_artifact_to_file(&debug_artifact, &circuit_name, circuit_dir); } -fn save_contract( - contract: CompiledContract, - package: &Package, - circuit_dir: &Path, - output_debug: bool, -) { +fn save_contract(contract: CompiledContract, package: &Package, circuit_dir: &Path) { // TODO(#1389): I wonder if it is incorrect for nargo-core to know anything about contracts. // As can be seen here, It seems like a leaky abstraction where ContractFunctions (essentially CompiledPrograms) // are compiled via nargo-core and then the PreprocessedContract is constructed here. @@ -323,13 +283,11 @@ fn save_contract( circuit_dir, ); - if output_debug { - save_debug_artifact_to_file( - &debug_artifact, - &format!("{}-{}", package.name, preprocessed_contract.name), - circuit_dir, - ); - } + save_debug_artifact_to_file( + &debug_artifact, + &format!("{}-{}", package.name, preprocessed_contract.name), + circuit_dir, + ); } /// Helper function for reporting any errors in a `CompilationResult` diff --git a/tooling/nargo_cli/src/cli/debug_cmd.rs b/tooling/nargo_cli/src/cli/debug_cmd.rs index 7d8c0dc9a15..82cd3349ec4 100644 --- a/tooling/nargo_cli/src/cli/debug_cmd.rs +++ b/tooling/nargo_cli/src/cli/debug_cmd.rs @@ -52,14 +52,10 @@ pub(crate) fn run( return Ok(()); }; - let compiled_program = compile_bin_package( - &workspace, - package, - &args.compile_options, - true, - np_language, - &|opcode| opcode_support.is_opcode_supported(opcode), - )?; + let compiled_program = + compile_bin_package(&workspace, package, &args.compile_options, np_language, &|opcode| { + opcode_support.is_opcode_supported(opcode) + })?; println!("[{}] Starting debugger", package.name); let (return_value, solved_witness) = diff --git a/tooling/nargo_cli/src/cli/execute_cmd.rs b/tooling/nargo_cli/src/cli/execute_cmd.rs index c61dc6db69c..1819c9a6f06 100644 --- a/tooling/nargo_cli/src/cli/execute_cmd.rs +++ b/tooling/nargo_cli/src/cli/execute_cmd.rs @@ -57,7 +57,6 @@ pub(crate) fn run( &workspace, package, &args.compile_options, - false, np_language, &|opcode| opcode_support.is_opcode_supported(opcode), )?; diff --git a/tooling/nargo_cli/src/cli/fs/program.rs b/tooling/nargo_cli/src/cli/fs/program.rs index 377786627be..e82f2d55264 100644 --- a/tooling/nargo_cli/src/cli/fs/program.rs +++ b/tooling/nargo_cli/src/cli/fs/program.rs @@ -60,3 +60,14 @@ pub(crate) fn read_program_from_file>( Ok(program) } + +pub(crate) fn read_debug_artifact_from_file>( + debug_artifact_path: P, +) -> Result { + let input_string = std::fs::read(&debug_artifact_path) + .map_err(|_| FilesystemError::PathNotValid(debug_artifact_path.as_ref().into()))?; + let program = serde_json::from_slice(&input_string) + .map_err(|err| FilesystemError::ProgramSerializationError(err.to_string()))?; + + Ok(program) +} diff --git a/tooling/nargo_cli/src/cli/info_cmd.rs b/tooling/nargo_cli/src/cli/info_cmd.rs index e55b1b7886f..50021e842c4 100644 --- a/tooling/nargo_cli/src/cli/info_cmd.rs +++ b/tooling/nargo_cli/src/cli/info_cmd.rs @@ -63,7 +63,6 @@ pub(crate) fn run( np_language, &opcode_support, &args.compile_options, - false, )?; let program_info = binary_packages diff --git a/tooling/nargo_cli/src/cli/prove_cmd.rs b/tooling/nargo_cli/src/cli/prove_cmd.rs index 5571117e2d4..af300b7ebe0 100644 --- a/tooling/nargo_cli/src/cli/prove_cmd.rs +++ b/tooling/nargo_cli/src/cli/prove_cmd.rs @@ -59,7 +59,6 @@ pub(crate) fn run( &workspace, package, &args.compile_options, - false, np_language, &|opcode| opcode_support.is_opcode_supported(opcode), )?; diff --git a/tooling/nargo_cli/src/cli/verify_cmd.rs b/tooling/nargo_cli/src/cli/verify_cmd.rs index a5a39e9aef9..6ae2b78fd0c 100644 --- a/tooling/nargo_cli/src/cli/verify_cmd.rs +++ b/tooling/nargo_cli/src/cli/verify_cmd.rs @@ -50,7 +50,6 @@ pub(crate) fn run( &workspace, package, &args.compile_options, - false, np_language, &|opcode| opcode_support.is_opcode_supported(opcode), )?;