Skip to content

Commit

Permalink
feat: Cache debug artifacts (#3133)
Browse files Browse the repository at this point in the history
Co-authored-by: kevaundray <kevtheappdev@gmail.com>
  • Loading branch information
TomAFrench and kevaundray authored Oct 23, 2023
1 parent 7e963e3 commit c5a6229
Show file tree
Hide file tree
Showing 8 changed files with 43 additions and 83 deletions.
1 change: 0 additions & 1 deletion tooling/nargo_cli/src/cli/codegen_verifier_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,6 @@ fn smart_contract_for_package(
workspace,
package,
compile_options,
false,
np_language,
&is_opcode_supported,
)?;
Expand Down
98 changes: 28 additions & 70 deletions tooling/nargo_cli/src/cli/compile_cmd.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use std::collections::BTreeMap;
use std::path::Path;

use acvm::acir::circuit::Opcode;
Expand All @@ -15,17 +14,16 @@ 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;

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::*;
Expand All @@ -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<CrateName>,
Expand Down Expand Up @@ -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(())
Expand All @@ -100,22 +93,14 @@ pub(super) fn compile_workspace(
np_language: Language,
opcode_support: &BackendOpcodeSupport,
compile_options: &CompileOptions,
output_debug: bool,
) -> Result<(Vec<CompiledProgram>, Vec<CompiledContract>), CliError> {
let is_opcode_supported = |opcode: &_| opcode_support.is_opcode_supported(opcode);

// Compile all of the packages in parallel.
let program_results: Vec<(FileManager, CompilationResult<CompiledProgram>)> = 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<CompiledContract>)> =
Expand Down Expand Up @@ -157,22 +142,15 @@ 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<CompiledProgram, CliError> {
if package.is_library() {
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,
Expand All @@ -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<CompiledProgram>) {
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) => {
Expand All @@ -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)))
}
Expand Down Expand Up @@ -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),
Expand All @@ -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.
Expand Down Expand Up @@ -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<T>`
Expand Down
12 changes: 4 additions & 8 deletions tooling/nargo_cli/src/cli/debug_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) =
Expand Down
1 change: 0 additions & 1 deletion tooling/nargo_cli/src/cli/execute_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ pub(crate) fn run(
&workspace,
package,
&args.compile_options,
false,
np_language,
&|opcode| opcode_support.is_opcode_supported(opcode),
)?;
Expand Down
11 changes: 11 additions & 0 deletions tooling/nargo_cli/src/cli/fs/program.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,3 +60,14 @@ pub(crate) fn read_program_from_file<P: AsRef<Path>>(

Ok(program)
}

pub(crate) fn read_debug_artifact_from_file<P: AsRef<Path>>(
debug_artifact_path: P,
) -> Result<DebugArtifact, FilesystemError> {
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)
}
1 change: 0 additions & 1 deletion tooling/nargo_cli/src/cli/info_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@ pub(crate) fn run(
np_language,
&opcode_support,
&args.compile_options,
false,
)?;

let program_info = binary_packages
Expand Down
1 change: 0 additions & 1 deletion tooling/nargo_cli/src/cli/prove_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ pub(crate) fn run(
&workspace,
package,
&args.compile_options,
false,
np_language,
&|opcode| opcode_support.is_opcode_supported(opcode),
)?;
Expand Down
1 change: 0 additions & 1 deletion tooling/nargo_cli/src/cli/verify_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ pub(crate) fn run(
&workspace,
package,
&args.compile_options,
false,
np_language,
&|opcode| opcode_support.is_opcode_supported(opcode),
)?;
Expand Down

0 comments on commit c5a6229

Please sign in to comment.