From af19dfc56d583e4b9b36bae042b861e5f715f90c Mon Sep 17 00:00:00 2001 From: Akosh Farkash Date: Wed, 9 Oct 2024 11:24:39 +0100 Subject: [PATCH 1/9] Turned nargo_cli into a lib+bin crate and made all commands public --- tooling/nargo_cli/Cargo.toml | 6 ++- tooling/nargo_cli/src/cli/check_cmd.rs | 10 ++-- tooling/nargo_cli/src/cli/compile_cmd.rs | 10 ++-- tooling/nargo_cli/src/cli/dap_cmd.rs | 16 +++---- tooling/nargo_cli/src/cli/debug_cmd.rs | 14 +++--- tooling/nargo_cli/src/cli/execute_cmd.rs | 14 +++--- tooling/nargo_cli/src/cli/export_cmd.rs | 8 ++-- tooling/nargo_cli/src/cli/fmt_cmd.rs | 4 +- tooling/nargo_cli/src/cli/info_cmd.rs | 12 ++--- tooling/nargo_cli/src/cli/init_cmd.rs | 10 ++-- tooling/nargo_cli/src/cli/lsp_cmd.rs | 2 +- tooling/nargo_cli/src/cli/mod.rs | 60 +++++++++++++----------- tooling/nargo_cli/src/cli/new_cmd.rs | 12 ++--- tooling/nargo_cli/src/cli/test_cmd.rs | 16 +++---- tooling/nargo_cli/src/lib.rs | 13 +++++ tooling/nargo_cli/src/main.rs | 14 +----- 16 files changed, 114 insertions(+), 107 deletions(-) create mode 100644 tooling/nargo_cli/src/lib.rs diff --git a/tooling/nargo_cli/Cargo.toml b/tooling/nargo_cli/Cargo.toml index acd5623871e..ef2cd548ebe 100644 --- a/tooling/nargo_cli/Cargo.toml +++ b/tooling/nargo_cli/Cargo.toml @@ -17,6 +17,7 @@ workspace = true [[bin]] name = "nargo" path = "src/main.rs" +required-features = ["build-binary"] [build-dependencies] build-data.workspace = true @@ -64,8 +65,8 @@ notify-debouncer-full = "0.3.1" termion = "3.0.0" # Logs -tracing-subscriber.workspace = true -tracing-appender = "0.2.3" +tracing-appender = { version = "0.2.3", optional = true } +tracing-subscriber = { workspace = true, optional = true } [target.'cfg(not(unix))'.dependencies] tokio-util = { version = "0.7.8", features = ["compat"] } @@ -97,3 +98,4 @@ harness = false [features] codegen-docs = ["dep:clap-markdown"] +build-binary = ["dep:tracing-subscriber", "dep:tracing-appender"] diff --git a/tooling/nargo_cli/src/cli/check_cmd.rs b/tooling/nargo_cli/src/cli/check_cmd.rs index 9059f1dd8e8..c99d437fbc8 100644 --- a/tooling/nargo_cli/src/cli/check_cmd.rs +++ b/tooling/nargo_cli/src/cli/check_cmd.rs @@ -23,21 +23,21 @@ use super::NargoConfig; /// Checks the constraint system for errors #[derive(Debug, Clone, Args)] #[clap(visible_alias = "c")] -pub(crate) struct CheckCommand { +pub struct CheckCommand { /// The name of the package to check #[clap(long, conflicts_with = "workspace")] - package: Option, + pub package: Option, /// Check all packages in the workspace #[clap(long, conflicts_with = "package")] - workspace: bool, + pub workspace: bool, /// Force overwrite of existing files #[clap(long = "overwrite")] - allow_overwrite: bool, + pub allow_overwrite: bool, #[clap(flatten)] - compile_options: CompileOptions, + pub compile_options: CompileOptions, } pub(crate) fn run(args: CheckCommand, config: NargoConfig) -> Result<(), CliError> { diff --git a/tooling/nargo_cli/src/cli/compile_cmd.rs b/tooling/nargo_cli/src/cli/compile_cmd.rs index 0ad07c91ff4..53c5361dbe9 100644 --- a/tooling/nargo_cli/src/cli/compile_cmd.rs +++ b/tooling/nargo_cli/src/cli/compile_cmd.rs @@ -26,21 +26,21 @@ use rayon::prelude::*; /// Compile the program and its secret execution trace into ACIR format #[derive(Debug, Clone, Args)] -pub(crate) struct CompileCommand { +pub struct CompileCommand { /// The name of the package to compile #[clap(long, conflicts_with = "workspace")] - package: Option, + pub package: Option, /// Compile all packages in the workspace. #[clap(long, conflicts_with = "package")] - workspace: bool, + pub workspace: bool, #[clap(flatten)] - compile_options: CompileOptions, + pub compile_options: CompileOptions, /// Watch workspace and recompile on changes. #[clap(long, hide = true)] - watch: bool, + pub watch: bool, } pub(crate) fn run(args: CompileCommand, config: NargoConfig) -> Result<(), CliError> { diff --git a/tooling/nargo_cli/src/cli/dap_cmd.rs b/tooling/nargo_cli/src/cli/dap_cmd.rs index a84e961cfe7..eaf695577f0 100644 --- a/tooling/nargo_cli/src/cli/dap_cmd.rs +++ b/tooling/nargo_cli/src/cli/dap_cmd.rs @@ -28,28 +28,28 @@ use super::NargoConfig; use noir_debugger::errors::{DapError, LoadError}; #[derive(Debug, Clone, Args)] -pub(crate) struct DapCommand { +pub struct DapCommand { /// Override the expression width requested by the backend. #[arg(long, value_parser = parse_expression_width, default_value = "4")] - expression_width: ExpressionWidth, + pub expression_width: ExpressionWidth, #[clap(long)] - preflight_check: bool, + pub preflight_check: bool, #[clap(long)] - preflight_project_folder: Option, + pub preflight_project_folder: Option, #[clap(long)] - preflight_package: Option, + pub preflight_package: Option, #[clap(long)] - preflight_prover_name: Option, + pub preflight_prover_name: Option, #[clap(long)] - preflight_generate_acir: bool, + pub preflight_generate_acir: bool, #[clap(long)] - preflight_skip_instrumentation: bool, + pub preflight_skip_instrumentation: bool, } fn parse_expression_width(input: &str) -> Result { diff --git a/tooling/nargo_cli/src/cli/debug_cmd.rs b/tooling/nargo_cli/src/cli/debug_cmd.rs index e837f297475..6b2170944db 100644 --- a/tooling/nargo_cli/src/cli/debug_cmd.rs +++ b/tooling/nargo_cli/src/cli/debug_cmd.rs @@ -28,28 +28,28 @@ use crate::errors::CliError; /// Executes a circuit in debug mode #[derive(Debug, Clone, Args)] -pub(crate) struct DebugCommand { +pub struct DebugCommand { /// Write the execution witness to named file - witness_name: Option, + pub witness_name: Option, /// The name of the toml file which contains the inputs for the prover #[clap(long, short, default_value = PROVER_INPUT_FILE)] - prover_name: String, + pub prover_name: String, /// The name of the package to execute #[clap(long)] - package: Option, + pub package: Option, #[clap(flatten)] - compile_options: CompileOptions, + pub compile_options: CompileOptions, /// Force ACIR output (disabling instrumentation) #[clap(long)] - acir_mode: bool, + pub acir_mode: bool, /// Disable vars debug instrumentation (enabled by default) #[clap(long)] - skip_instrumentation: Option, + pub skip_instrumentation: Option, } pub(crate) fn run(args: DebugCommand, config: NargoConfig) -> Result<(), CliError> { diff --git a/tooling/nargo_cli/src/cli/execute_cmd.rs b/tooling/nargo_cli/src/cli/execute_cmd.rs index 8dc71b1c7e5..625de0aedcc 100644 --- a/tooling/nargo_cli/src/cli/execute_cmd.rs +++ b/tooling/nargo_cli/src/cli/execute_cmd.rs @@ -24,30 +24,30 @@ use crate::errors::CliError; /// Executes a circuit to calculate its return value #[derive(Debug, Clone, Args)] #[clap(visible_alias = "e")] -pub(crate) struct ExecuteCommand { +pub struct ExecuteCommand { /// Write the execution witness to named file /// /// Defaults to the name of the package being executed. - witness_name: Option, + pub witness_name: Option, /// The name of the toml file which contains the inputs for the prover #[clap(long, short, default_value = PROVER_INPUT_FILE)] - prover_name: String, + pub prover_name: String, /// The name of the package to execute #[clap(long, conflicts_with = "workspace")] - package: Option, + pub package: Option, /// Execute all packages in the workspace #[clap(long, conflicts_with = "package")] - workspace: bool, + pub workspace: bool, #[clap(flatten)] - compile_options: CompileOptions, + pub compile_options: CompileOptions, /// JSON RPC url to solve oracle calls #[clap(long)] - oracle_resolver: Option, + pub oracle_resolver: Option, } pub(crate) fn run(args: ExecuteCommand, config: NargoConfig) -> Result<(), CliError> { diff --git a/tooling/nargo_cli/src/cli/export_cmd.rs b/tooling/nargo_cli/src/cli/export_cmd.rs index c3752db7fbd..54a4b2471bd 100644 --- a/tooling/nargo_cli/src/cli/export_cmd.rs +++ b/tooling/nargo_cli/src/cli/export_cmd.rs @@ -28,17 +28,17 @@ use super::NargoConfig; /// Exports functions marked with #[export] attribute #[derive(Debug, Clone, Args)] -pub(crate) struct ExportCommand { +pub struct ExportCommand { /// The name of the package to compile #[clap(long, conflicts_with = "workspace")] - package: Option, + pub package: Option, /// Compile all packages in the workspace #[clap(long, conflicts_with = "package")] - workspace: bool, + pub workspace: bool, #[clap(flatten)] - compile_options: CompileOptions, + pub compile_options: CompileOptions, } pub(crate) fn run(args: ExportCommand, config: NargoConfig) -> Result<(), CliError> { diff --git a/tooling/nargo_cli/src/cli/fmt_cmd.rs b/tooling/nargo_cli/src/cli/fmt_cmd.rs index 66496db517c..69788ae383e 100644 --- a/tooling/nargo_cli/src/cli/fmt_cmd.rs +++ b/tooling/nargo_cli/src/cli/fmt_cmd.rs @@ -13,10 +13,10 @@ use super::NargoConfig; /// Format the Noir files in a workspace #[derive(Debug, Clone, Args)] -pub(crate) struct FormatCommand { +pub struct FormatCommand { /// Run noirfmt in check mode #[arg(long)] - check: bool, + pub check: bool, } pub(crate) fn run(args: FormatCommand, config: NargoConfig) -> Result<(), CliError> { diff --git a/tooling/nargo_cli/src/cli/info_cmd.rs b/tooling/nargo_cli/src/cli/info_cmd.rs index f2e44fa893c..93230891925 100644 --- a/tooling/nargo_cli/src/cli/info_cmd.rs +++ b/tooling/nargo_cli/src/cli/info_cmd.rs @@ -27,24 +27,24 @@ use super::{ /// 2. Counts the final number gates in the circuit used by a backend #[derive(Debug, Clone, Args)] #[clap(visible_alias = "i")] -pub(crate) struct InfoCommand { +pub struct InfoCommand { /// The name of the package to detail #[clap(long, conflicts_with = "workspace")] - package: Option, + pub package: Option, /// Detail all packages in the workspace #[clap(long, conflicts_with = "package")] - workspace: bool, + pub workspace: bool, /// Output a JSON formatted report. Changes to this format are not currently considered breaking. #[clap(long, hide = true)] - json: bool, + pub json: bool, #[clap(long, hide = true)] - profile_info: bool, + pub profile_info: bool, #[clap(flatten)] - compile_options: CompileOptions, + pub compile_options: CompileOptions, } pub(crate) fn run(args: InfoCommand, config: NargoConfig) -> Result<(), CliError> { diff --git a/tooling/nargo_cli/src/cli/init_cmd.rs b/tooling/nargo_cli/src/cli/init_cmd.rs index c69775d3323..e9326b9352c 100644 --- a/tooling/nargo_cli/src/cli/init_cmd.rs +++ b/tooling/nargo_cli/src/cli/init_cmd.rs @@ -10,22 +10,22 @@ use std::path::PathBuf; /// Create a Noir project in the current directory. #[derive(Debug, Clone, Args)] -pub(crate) struct InitCommand { +pub struct InitCommand { /// Name of the package [default: current directory name] #[clap(long)] - name: Option, + pub name: Option, /// Use a library template #[arg(long, conflicts_with = "bin", conflicts_with = "contract")] - pub(crate) lib: bool, + pub lib: bool, /// Use a binary template [default] #[arg(long, conflicts_with = "lib", conflicts_with = "contract")] - pub(crate) bin: bool, + pub bin: bool, /// Use a contract template #[arg(long, conflicts_with = "lib", conflicts_with = "bin")] - pub(crate) contract: bool, + pub contract: bool, } const BIN_EXAMPLE: &str = include_str!("./noir_template_files/binary.nr"); diff --git a/tooling/nargo_cli/src/cli/lsp_cmd.rs b/tooling/nargo_cli/src/cli/lsp_cmd.rs index bfaa913b33a..10fa55b83e1 100644 --- a/tooling/nargo_cli/src/cli/lsp_cmd.rs +++ b/tooling/nargo_cli/src/cli/lsp_cmd.rs @@ -16,7 +16,7 @@ use crate::errors::CliError; /// /// VS Code Noir Language Support: https://marketplace.visualstudio.com/items?itemName=noir-lang.vscode-noir #[derive(Debug, Clone, Args)] -pub(crate) struct LspCommand; +pub struct LspCommand; pub(crate) fn run(_args: LspCommand, _config: NargoConfig) -> Result<(), CliError> { use tokio::runtime::Builder; diff --git a/tooling/nargo_cli/src/cli/mod.rs b/tooling/nargo_cli/src/cli/mod.rs index 10ec38ad1d5..d3812d05769 100644 --- a/tooling/nargo_cli/src/cli/mod.rs +++ b/tooling/nargo_cli/src/cli/mod.rs @@ -8,18 +8,18 @@ use color_eyre::eyre; mod fs; -mod check_cmd; -mod compile_cmd; -mod dap_cmd; -mod debug_cmd; -mod execute_cmd; -mod export_cmd; -mod fmt_cmd; -mod info_cmd; -mod init_cmd; -mod lsp_cmd; -mod new_cmd; -mod test_cmd; +pub mod check_cmd; +pub mod compile_cmd; +pub mod dap_cmd; +pub mod debug_cmd; +pub mod execute_cmd; +pub mod export_cmd; +pub mod fmt_cmd; +pub mod info_cmd; +pub mod init_cmd; +pub mod lsp_cmd; +pub mod new_cmd; +pub mod test_cmd; const GIT_HASH: &str = env!("GIT_COMMIT"); const IS_DIRTY: &str = env!("GIT_DIRTY"); @@ -35,25 +35,23 @@ static VERSION_STRING: &str = formatcp!( #[derive(Parser, Debug)] #[command(name="nargo", author, version=VERSION_STRING, about, long_about = None)] -struct NargoCli { +pub struct NargoCli { #[command(subcommand)] - command: NargoCommand, + pub command: NargoCommand, #[clap(flatten)] - config: NargoConfig, + pub config: NargoConfig, } -#[non_exhaustive] #[derive(Args, Clone, Debug)] -pub(crate) struct NargoConfig { +pub struct NargoConfig { // REMINDER: Also change this flag in the LSP test lens if renamed #[arg(long, hide = true, global = true, default_value = "./")] - program_dir: PathBuf, + pub program_dir: PathBuf, } -#[non_exhaustive] #[derive(Subcommand, Clone, Debug)] -enum NargoCommand { +pub enum NargoCommand { Check(check_cmd::CheckCommand), Fmt(fmt_cmd::FormatCommand), #[command(alias = "build")] @@ -71,9 +69,22 @@ enum NargoCommand { Dap(dap_cmd::DapCommand), } +/// Parse the command line arguments and execute the command. #[cfg(not(feature = "codegen-docs"))] +pub fn start_cli() -> eyre::Result<()> { + run_cmd(NargoCli::parse()) +} + +#[cfg(feature = "codegen-docs")] pub(crate) fn start_cli() -> eyre::Result<()> { - let NargoCli { command, mut config } = NargoCli::parse(); + let markdown: String = clap_markdown::help_markdown::(); + println!("{markdown}"); + Ok(()) +} + +/// Execute the CLI command. +pub fn run_cmd(cmd: NargoCli) -> eyre::Result<()> { + let NargoCli { command, mut config } = cmd; // If the provided `program_dir` is relative, make it absolute by joining it to the current directory. if !config.program_dir.is_absolute() { @@ -105,10 +116,3 @@ pub(crate) fn start_cli() -> eyre::Result<()> { Ok(()) } - -#[cfg(feature = "codegen-docs")] -pub(crate) fn start_cli() -> eyre::Result<()> { - let markdown: String = clap_markdown::help_markdown::(); - println!("{markdown}"); - Ok(()) -} diff --git a/tooling/nargo_cli/src/cli/new_cmd.rs b/tooling/nargo_cli/src/cli/new_cmd.rs index db9257b8aa0..f8a9d206cbd 100644 --- a/tooling/nargo_cli/src/cli/new_cmd.rs +++ b/tooling/nargo_cli/src/cli/new_cmd.rs @@ -7,25 +7,25 @@ use std::path::PathBuf; /// Create a Noir project in a new directory. #[derive(Debug, Clone, Args)] -pub(crate) struct NewCommand { +pub struct NewCommand { /// The path to save the new project - path: PathBuf, + pub path: PathBuf, /// Name of the package [default: package directory name] #[clap(long)] - name: Option, + pub name: Option, /// Use a library template #[arg(long, conflicts_with = "bin", conflicts_with = "contract")] - pub(crate) lib: bool, + pub lib: bool, /// Use a binary template [default] #[arg(long, conflicts_with = "lib", conflicts_with = "contract")] - pub(crate) bin: bool, + pub bin: bool, /// Use a contract template #[arg(long, conflicts_with = "lib", conflicts_with = "bin")] - pub(crate) contract: bool, + pub contract: bool, } pub(crate) fn run(args: NewCommand, config: NargoConfig) -> Result<(), CliError> { diff --git a/tooling/nargo_cli/src/cli/test_cmd.rs b/tooling/nargo_cli/src/cli/test_cmd.rs index 7b0201226ef..13726629d82 100644 --- a/tooling/nargo_cli/src/cli/test_cmd.rs +++ b/tooling/nargo_cli/src/cli/test_cmd.rs @@ -23,32 +23,32 @@ use super::NargoConfig; /// Run the tests for this program #[derive(Debug, Clone, Args)] #[clap(visible_alias = "t")] -pub(crate) struct TestCommand { +pub struct TestCommand { /// If given, only tests with names containing this string will be run - test_name: Option, + pub test_name: Option, /// Display output of `println` statements #[arg(long)] - show_output: bool, + pub show_output: bool, /// Only run tests that match exactly #[clap(long)] - exact: bool, + pub exact: bool, /// The name of the package to test #[clap(long, conflicts_with = "workspace")] - package: Option, + pub package: Option, /// Test all packages in the workspace #[clap(long, conflicts_with = "package")] - workspace: bool, + pub workspace: bool, #[clap(flatten)] - compile_options: CompileOptions, + pub compile_options: CompileOptions, /// JSON RPC url to solve oracle calls #[clap(long)] - oracle_resolver: Option, + pub oracle_resolver: Option, } pub(crate) fn run(args: TestCommand, config: NargoConfig) -> Result<(), CliError> { diff --git a/tooling/nargo_cli/src/lib.rs b/tooling/nargo_cli/src/lib.rs new file mode 100644 index 00000000000..5bd20108439 --- /dev/null +++ b/tooling/nargo_cli/src/lib.rs @@ -0,0 +1,13 @@ +#![forbid(unsafe_code)] +#![warn(unreachable_pub)] +#![warn(clippy::semicolon_if_nothing_returned)] +#![cfg_attr(not(test), warn(unused_crate_dependencies, unused_extern_crates))] + +//! Nargo is the package manager for Noir +//! This name was used because it sounds like `cargo` and +//! Noir Package Manager abbreviated is npm, which is already taken. + +pub mod cli; +mod errors; + +pub const PANIC_MESSAGE: &str = "This is a bug. We may have already fixed this in newer versions of Nargo so try searching for similar issues at https://github.com/noir-lang/noir/issues/.\nIf there isn't an open issue for this bug, consider opening one at https://github.com/noir-lang/noir/issues/new?labels=bug&template=bug_report.yml"; diff --git a/tooling/nargo_cli/src/main.rs b/tooling/nargo_cli/src/main.rs index a407d467ced..da274a27f38 100644 --- a/tooling/nargo_cli/src/main.rs +++ b/tooling/nargo_cli/src/main.rs @@ -1,15 +1,3 @@ -#![forbid(unsafe_code)] -#![warn(unreachable_pub)] -#![warn(clippy::semicolon_if_nothing_returned)] -#![cfg_attr(not(test), warn(unused_crate_dependencies, unused_extern_crates))] - -//! Nargo is the package manager for Noir -//! This name was used because it sounds like `cargo` and -//! Noir Package Manager abbreviated is npm, which is already taken. - -mod cli; -mod errors; - use std::env; use color_eyre::config::HookBuilder; @@ -17,7 +5,7 @@ use color_eyre::config::HookBuilder; use tracing_appender::rolling; use tracing_subscriber::{fmt::format::FmtSpan, EnvFilter}; -const PANIC_MESSAGE: &str = "This is a bug. We may have already fixed this in newer versions of Nargo so try searching for similar issues at https://github.com/noir-lang/noir/issues/.\nIf there isn't an open issue for this bug, consider opening one at https://github.com/noir-lang/noir/issues/new?labels=bug&template=bug_report.yml"; +use nargo_cli::{cli, PANIC_MESSAGE}; fn main() { // Setup tracing From bb269a03d003372c71162f1561df88445d81df7d Mon Sep 17 00:00:00 2001 From: Akosh Farkash Date: Wed, 9 Oct 2024 11:25:17 +0100 Subject: [PATCH 2/9] Split benchmarking into a compile and an execution step --- tooling/nargo_cli/benches/README.md | 13 +++++ tooling/nargo_cli/benches/criterion.rs | 66 ++++++++++++++++++-------- 2 files changed, 59 insertions(+), 20 deletions(-) create mode 100644 tooling/nargo_cli/benches/README.md diff --git a/tooling/nargo_cli/benches/README.md b/tooling/nargo_cli/benches/README.md new file mode 100644 index 00000000000..a579b82bbb5 --- /dev/null +++ b/tooling/nargo_cli/benches/README.md @@ -0,0 +1,13 @@ +# Benchmarks + +To generate flamegraphs for the execution of a specific program, execute the following commands: + +```shell +./scripts/benchmark_start.sh +cargo bench -p nargo_cli --bench criterion -- --profile-time=30 +./scripts/benchmark_stop.sh +``` + +Afterwards the flamegraph is available at `target/criterion/_execute/profile/flamegraph.svg` + +Alternatively, omit `` to run profiling on all test programs defined in [utils.rs](./utils.rs). \ No newline at end of file diff --git a/tooling/nargo_cli/benches/criterion.rs b/tooling/nargo_cli/benches/criterion.rs index effab7d7c27..82cc0fe2137 100644 --- a/tooling/nargo_cli/benches/criterion.rs +++ b/tooling/nargo_cli/benches/criterion.rs @@ -2,32 +2,58 @@ use assert_cmd::prelude::{CommandCargoExt, OutputAssertExt}; use criterion::{criterion_group, criterion_main, Criterion}; -use paste::paste; +use nargo_cli::cli; +use noirc_driver::CompileOptions; use pprof::criterion::{Output, PProfProfiler}; use std::{process::Command, time::Duration}; include!("./utils.rs"); -macro_rules! criterion_command { - ($command_name:tt, $command_string:expr) => { - paste! { - fn [](c: &mut Criterion) { - let test_program_dirs = get_selected_tests(); - for test_program_dir in test_program_dirs { - let mut cmd = Command::cargo_bin("nargo").unwrap(); - cmd.arg("--program-dir").arg(&test_program_dir); - cmd.arg($command_string); - cmd.arg("--force"); +/// Use the nargo CLI to compile a test program, then benchmark its execution +/// by executing the command directly from the benchmark, so that we can have +/// meaningful flamegraphs about the ACVM. +fn criterion_selected_tests_execution(c: &mut Criterion) { + for test_program_dir in get_selected_tests() { + let mut compiled = false; + let benchmark_name = + format!("{}_execute", test_program_dir.file_name().unwrap().to_str().unwrap()); - let benchmark_name = format!("{}_{}", test_program_dir.file_name().unwrap().to_str().unwrap(), $command_string); - c.bench_function(&benchmark_name, |b| { - b.iter(|| cmd.assert().success()) - }); - } - } - } - }; + c.bench_function(&benchmark_name, |b| { + b.iter_batched( + || { + // Setup will be called many times to set a batch (which we don't use), + // but we can compile it only once, and then the executions will not have to do so. + // It is done as a setup so that we only compile the test programs that we filter for. + if !compiled { + compiled = true; + let mut cmd = Command::cargo_bin("nargo").unwrap(); + cmd.arg("--program-dir").arg(&test_program_dir); + cmd.arg("compile"); + cmd.arg("--force"); + cmd.assert().success(); + } + }, + |_| { + let cmd = cli::NargoCli { + command: cli::NargoCommand::Execute(cli::execute_cmd::ExecuteCommand { + witness_name: None, + prover_name: nargo::constants::PROVER_INPUT_FILE.to_string(), + package: None, + workspace: true, + compile_options: CompileOptions { + silence_warnings: true, + ..Default::default() + }, + oracle_resolver: None, + }), + config: cli::NargoConfig { program_dir: test_program_dir.clone() }, + }; + cli::run_cmd(cmd).expect("failed to execute command"); + }, + criterion::BatchSize::SmallInput, + ); + }); + } } -criterion_command!(execution, "execute"); criterion_group! { name = execution_benches; From 37855a3c8d36ea06e56a0b7bb809245f0f9568e1 Mon Sep 17 00:00:00 2001 From: Akosh Farkash Date: Wed, 9 Oct 2024 12:39:16 +0100 Subject: [PATCH 3/9] Revert "Turned nargo_cli into a lib+bin crate and made all commands public" This reverts commit af19dfc56d583e4b9b36bae042b861e5f715f90c. --- tooling/nargo_cli/Cargo.toml | 6 +-- tooling/nargo_cli/src/cli/check_cmd.rs | 10 ++-- tooling/nargo_cli/src/cli/compile_cmd.rs | 10 ++-- tooling/nargo_cli/src/cli/dap_cmd.rs | 16 +++---- tooling/nargo_cli/src/cli/debug_cmd.rs | 14 +++--- tooling/nargo_cli/src/cli/execute_cmd.rs | 14 +++--- tooling/nargo_cli/src/cli/export_cmd.rs | 8 ++-- tooling/nargo_cli/src/cli/fmt_cmd.rs | 4 +- tooling/nargo_cli/src/cli/info_cmd.rs | 12 ++--- tooling/nargo_cli/src/cli/init_cmd.rs | 10 ++-- tooling/nargo_cli/src/cli/lsp_cmd.rs | 2 +- tooling/nargo_cli/src/cli/mod.rs | 60 +++++++++++------------- tooling/nargo_cli/src/cli/new_cmd.rs | 12 ++--- tooling/nargo_cli/src/cli/test_cmd.rs | 16 +++---- tooling/nargo_cli/src/lib.rs | 13 ----- tooling/nargo_cli/src/main.rs | 14 +++++- 16 files changed, 107 insertions(+), 114 deletions(-) delete mode 100644 tooling/nargo_cli/src/lib.rs diff --git a/tooling/nargo_cli/Cargo.toml b/tooling/nargo_cli/Cargo.toml index ef2cd548ebe..acd5623871e 100644 --- a/tooling/nargo_cli/Cargo.toml +++ b/tooling/nargo_cli/Cargo.toml @@ -17,7 +17,6 @@ workspace = true [[bin]] name = "nargo" path = "src/main.rs" -required-features = ["build-binary"] [build-dependencies] build-data.workspace = true @@ -65,8 +64,8 @@ notify-debouncer-full = "0.3.1" termion = "3.0.0" # Logs -tracing-appender = { version = "0.2.3", optional = true } -tracing-subscriber = { workspace = true, optional = true } +tracing-subscriber.workspace = true +tracing-appender = "0.2.3" [target.'cfg(not(unix))'.dependencies] tokio-util = { version = "0.7.8", features = ["compat"] } @@ -98,4 +97,3 @@ harness = false [features] codegen-docs = ["dep:clap-markdown"] -build-binary = ["dep:tracing-subscriber", "dep:tracing-appender"] diff --git a/tooling/nargo_cli/src/cli/check_cmd.rs b/tooling/nargo_cli/src/cli/check_cmd.rs index c99d437fbc8..9059f1dd8e8 100644 --- a/tooling/nargo_cli/src/cli/check_cmd.rs +++ b/tooling/nargo_cli/src/cli/check_cmd.rs @@ -23,21 +23,21 @@ use super::NargoConfig; /// Checks the constraint system for errors #[derive(Debug, Clone, Args)] #[clap(visible_alias = "c")] -pub struct CheckCommand { +pub(crate) struct CheckCommand { /// The name of the package to check #[clap(long, conflicts_with = "workspace")] - pub package: Option, + package: Option, /// Check all packages in the workspace #[clap(long, conflicts_with = "package")] - pub workspace: bool, + workspace: bool, /// Force overwrite of existing files #[clap(long = "overwrite")] - pub allow_overwrite: bool, + allow_overwrite: bool, #[clap(flatten)] - pub compile_options: CompileOptions, + compile_options: CompileOptions, } pub(crate) fn run(args: CheckCommand, config: NargoConfig) -> Result<(), CliError> { diff --git a/tooling/nargo_cli/src/cli/compile_cmd.rs b/tooling/nargo_cli/src/cli/compile_cmd.rs index 53c5361dbe9..0ad07c91ff4 100644 --- a/tooling/nargo_cli/src/cli/compile_cmd.rs +++ b/tooling/nargo_cli/src/cli/compile_cmd.rs @@ -26,21 +26,21 @@ use rayon::prelude::*; /// Compile the program and its secret execution trace into ACIR format #[derive(Debug, Clone, Args)] -pub struct CompileCommand { +pub(crate) struct CompileCommand { /// The name of the package to compile #[clap(long, conflicts_with = "workspace")] - pub package: Option, + package: Option, /// Compile all packages in the workspace. #[clap(long, conflicts_with = "package")] - pub workspace: bool, + workspace: bool, #[clap(flatten)] - pub compile_options: CompileOptions, + compile_options: CompileOptions, /// Watch workspace and recompile on changes. #[clap(long, hide = true)] - pub watch: bool, + watch: bool, } pub(crate) fn run(args: CompileCommand, config: NargoConfig) -> Result<(), CliError> { diff --git a/tooling/nargo_cli/src/cli/dap_cmd.rs b/tooling/nargo_cli/src/cli/dap_cmd.rs index eaf695577f0..a84e961cfe7 100644 --- a/tooling/nargo_cli/src/cli/dap_cmd.rs +++ b/tooling/nargo_cli/src/cli/dap_cmd.rs @@ -28,28 +28,28 @@ use super::NargoConfig; use noir_debugger::errors::{DapError, LoadError}; #[derive(Debug, Clone, Args)] -pub struct DapCommand { +pub(crate) struct DapCommand { /// Override the expression width requested by the backend. #[arg(long, value_parser = parse_expression_width, default_value = "4")] - pub expression_width: ExpressionWidth, + expression_width: ExpressionWidth, #[clap(long)] - pub preflight_check: bool, + preflight_check: bool, #[clap(long)] - pub preflight_project_folder: Option, + preflight_project_folder: Option, #[clap(long)] - pub preflight_package: Option, + preflight_package: Option, #[clap(long)] - pub preflight_prover_name: Option, + preflight_prover_name: Option, #[clap(long)] - pub preflight_generate_acir: bool, + preflight_generate_acir: bool, #[clap(long)] - pub preflight_skip_instrumentation: bool, + preflight_skip_instrumentation: bool, } fn parse_expression_width(input: &str) -> Result { diff --git a/tooling/nargo_cli/src/cli/debug_cmd.rs b/tooling/nargo_cli/src/cli/debug_cmd.rs index 6b2170944db..e837f297475 100644 --- a/tooling/nargo_cli/src/cli/debug_cmd.rs +++ b/tooling/nargo_cli/src/cli/debug_cmd.rs @@ -28,28 +28,28 @@ use crate::errors::CliError; /// Executes a circuit in debug mode #[derive(Debug, Clone, Args)] -pub struct DebugCommand { +pub(crate) struct DebugCommand { /// Write the execution witness to named file - pub witness_name: Option, + witness_name: Option, /// The name of the toml file which contains the inputs for the prover #[clap(long, short, default_value = PROVER_INPUT_FILE)] - pub prover_name: String, + prover_name: String, /// The name of the package to execute #[clap(long)] - pub package: Option, + package: Option, #[clap(flatten)] - pub compile_options: CompileOptions, + compile_options: CompileOptions, /// Force ACIR output (disabling instrumentation) #[clap(long)] - pub acir_mode: bool, + acir_mode: bool, /// Disable vars debug instrumentation (enabled by default) #[clap(long)] - pub skip_instrumentation: Option, + skip_instrumentation: Option, } pub(crate) fn run(args: DebugCommand, config: NargoConfig) -> Result<(), CliError> { diff --git a/tooling/nargo_cli/src/cli/execute_cmd.rs b/tooling/nargo_cli/src/cli/execute_cmd.rs index 625de0aedcc..8dc71b1c7e5 100644 --- a/tooling/nargo_cli/src/cli/execute_cmd.rs +++ b/tooling/nargo_cli/src/cli/execute_cmd.rs @@ -24,30 +24,30 @@ use crate::errors::CliError; /// Executes a circuit to calculate its return value #[derive(Debug, Clone, Args)] #[clap(visible_alias = "e")] -pub struct ExecuteCommand { +pub(crate) struct ExecuteCommand { /// Write the execution witness to named file /// /// Defaults to the name of the package being executed. - pub witness_name: Option, + witness_name: Option, /// The name of the toml file which contains the inputs for the prover #[clap(long, short, default_value = PROVER_INPUT_FILE)] - pub prover_name: String, + prover_name: String, /// The name of the package to execute #[clap(long, conflicts_with = "workspace")] - pub package: Option, + package: Option, /// Execute all packages in the workspace #[clap(long, conflicts_with = "package")] - pub workspace: bool, + workspace: bool, #[clap(flatten)] - pub compile_options: CompileOptions, + compile_options: CompileOptions, /// JSON RPC url to solve oracle calls #[clap(long)] - pub oracle_resolver: Option, + oracle_resolver: Option, } pub(crate) fn run(args: ExecuteCommand, config: NargoConfig) -> Result<(), CliError> { diff --git a/tooling/nargo_cli/src/cli/export_cmd.rs b/tooling/nargo_cli/src/cli/export_cmd.rs index 54a4b2471bd..c3752db7fbd 100644 --- a/tooling/nargo_cli/src/cli/export_cmd.rs +++ b/tooling/nargo_cli/src/cli/export_cmd.rs @@ -28,17 +28,17 @@ use super::NargoConfig; /// Exports functions marked with #[export] attribute #[derive(Debug, Clone, Args)] -pub struct ExportCommand { +pub(crate) struct ExportCommand { /// The name of the package to compile #[clap(long, conflicts_with = "workspace")] - pub package: Option, + package: Option, /// Compile all packages in the workspace #[clap(long, conflicts_with = "package")] - pub workspace: bool, + workspace: bool, #[clap(flatten)] - pub compile_options: CompileOptions, + compile_options: CompileOptions, } pub(crate) fn run(args: ExportCommand, config: NargoConfig) -> Result<(), CliError> { diff --git a/tooling/nargo_cli/src/cli/fmt_cmd.rs b/tooling/nargo_cli/src/cli/fmt_cmd.rs index 69788ae383e..66496db517c 100644 --- a/tooling/nargo_cli/src/cli/fmt_cmd.rs +++ b/tooling/nargo_cli/src/cli/fmt_cmd.rs @@ -13,10 +13,10 @@ use super::NargoConfig; /// Format the Noir files in a workspace #[derive(Debug, Clone, Args)] -pub struct FormatCommand { +pub(crate) struct FormatCommand { /// Run noirfmt in check mode #[arg(long)] - pub check: bool, + check: bool, } pub(crate) fn run(args: FormatCommand, config: NargoConfig) -> Result<(), CliError> { diff --git a/tooling/nargo_cli/src/cli/info_cmd.rs b/tooling/nargo_cli/src/cli/info_cmd.rs index 93230891925..f2e44fa893c 100644 --- a/tooling/nargo_cli/src/cli/info_cmd.rs +++ b/tooling/nargo_cli/src/cli/info_cmd.rs @@ -27,24 +27,24 @@ use super::{ /// 2. Counts the final number gates in the circuit used by a backend #[derive(Debug, Clone, Args)] #[clap(visible_alias = "i")] -pub struct InfoCommand { +pub(crate) struct InfoCommand { /// The name of the package to detail #[clap(long, conflicts_with = "workspace")] - pub package: Option, + package: Option, /// Detail all packages in the workspace #[clap(long, conflicts_with = "package")] - pub workspace: bool, + workspace: bool, /// Output a JSON formatted report. Changes to this format are not currently considered breaking. #[clap(long, hide = true)] - pub json: bool, + json: bool, #[clap(long, hide = true)] - pub profile_info: bool, + profile_info: bool, #[clap(flatten)] - pub compile_options: CompileOptions, + compile_options: CompileOptions, } pub(crate) fn run(args: InfoCommand, config: NargoConfig) -> Result<(), CliError> { diff --git a/tooling/nargo_cli/src/cli/init_cmd.rs b/tooling/nargo_cli/src/cli/init_cmd.rs index e9326b9352c..c69775d3323 100644 --- a/tooling/nargo_cli/src/cli/init_cmd.rs +++ b/tooling/nargo_cli/src/cli/init_cmd.rs @@ -10,22 +10,22 @@ use std::path::PathBuf; /// Create a Noir project in the current directory. #[derive(Debug, Clone, Args)] -pub struct InitCommand { +pub(crate) struct InitCommand { /// Name of the package [default: current directory name] #[clap(long)] - pub name: Option, + name: Option, /// Use a library template #[arg(long, conflicts_with = "bin", conflicts_with = "contract")] - pub lib: bool, + pub(crate) lib: bool, /// Use a binary template [default] #[arg(long, conflicts_with = "lib", conflicts_with = "contract")] - pub bin: bool, + pub(crate) bin: bool, /// Use a contract template #[arg(long, conflicts_with = "lib", conflicts_with = "bin")] - pub contract: bool, + pub(crate) contract: bool, } const BIN_EXAMPLE: &str = include_str!("./noir_template_files/binary.nr"); diff --git a/tooling/nargo_cli/src/cli/lsp_cmd.rs b/tooling/nargo_cli/src/cli/lsp_cmd.rs index 10fa55b83e1..bfaa913b33a 100644 --- a/tooling/nargo_cli/src/cli/lsp_cmd.rs +++ b/tooling/nargo_cli/src/cli/lsp_cmd.rs @@ -16,7 +16,7 @@ use crate::errors::CliError; /// /// VS Code Noir Language Support: https://marketplace.visualstudio.com/items?itemName=noir-lang.vscode-noir #[derive(Debug, Clone, Args)] -pub struct LspCommand; +pub(crate) struct LspCommand; pub(crate) fn run(_args: LspCommand, _config: NargoConfig) -> Result<(), CliError> { use tokio::runtime::Builder; diff --git a/tooling/nargo_cli/src/cli/mod.rs b/tooling/nargo_cli/src/cli/mod.rs index d3812d05769..10ec38ad1d5 100644 --- a/tooling/nargo_cli/src/cli/mod.rs +++ b/tooling/nargo_cli/src/cli/mod.rs @@ -8,18 +8,18 @@ use color_eyre::eyre; mod fs; -pub mod check_cmd; -pub mod compile_cmd; -pub mod dap_cmd; -pub mod debug_cmd; -pub mod execute_cmd; -pub mod export_cmd; -pub mod fmt_cmd; -pub mod info_cmd; -pub mod init_cmd; -pub mod lsp_cmd; -pub mod new_cmd; -pub mod test_cmd; +mod check_cmd; +mod compile_cmd; +mod dap_cmd; +mod debug_cmd; +mod execute_cmd; +mod export_cmd; +mod fmt_cmd; +mod info_cmd; +mod init_cmd; +mod lsp_cmd; +mod new_cmd; +mod test_cmd; const GIT_HASH: &str = env!("GIT_COMMIT"); const IS_DIRTY: &str = env!("GIT_DIRTY"); @@ -35,23 +35,25 @@ static VERSION_STRING: &str = formatcp!( #[derive(Parser, Debug)] #[command(name="nargo", author, version=VERSION_STRING, about, long_about = None)] -pub struct NargoCli { +struct NargoCli { #[command(subcommand)] - pub command: NargoCommand, + command: NargoCommand, #[clap(flatten)] - pub config: NargoConfig, + config: NargoConfig, } +#[non_exhaustive] #[derive(Args, Clone, Debug)] -pub struct NargoConfig { +pub(crate) struct NargoConfig { // REMINDER: Also change this flag in the LSP test lens if renamed #[arg(long, hide = true, global = true, default_value = "./")] - pub program_dir: PathBuf, + program_dir: PathBuf, } +#[non_exhaustive] #[derive(Subcommand, Clone, Debug)] -pub enum NargoCommand { +enum NargoCommand { Check(check_cmd::CheckCommand), Fmt(fmt_cmd::FormatCommand), #[command(alias = "build")] @@ -69,22 +71,9 @@ pub enum NargoCommand { Dap(dap_cmd::DapCommand), } -/// Parse the command line arguments and execute the command. #[cfg(not(feature = "codegen-docs"))] -pub fn start_cli() -> eyre::Result<()> { - run_cmd(NargoCli::parse()) -} - -#[cfg(feature = "codegen-docs")] pub(crate) fn start_cli() -> eyre::Result<()> { - let markdown: String = clap_markdown::help_markdown::(); - println!("{markdown}"); - Ok(()) -} - -/// Execute the CLI command. -pub fn run_cmd(cmd: NargoCli) -> eyre::Result<()> { - let NargoCli { command, mut config } = cmd; + let NargoCli { command, mut config } = NargoCli::parse(); // If the provided `program_dir` is relative, make it absolute by joining it to the current directory. if !config.program_dir.is_absolute() { @@ -116,3 +105,10 @@ pub fn run_cmd(cmd: NargoCli) -> eyre::Result<()> { Ok(()) } + +#[cfg(feature = "codegen-docs")] +pub(crate) fn start_cli() -> eyre::Result<()> { + let markdown: String = clap_markdown::help_markdown::(); + println!("{markdown}"); + Ok(()) +} diff --git a/tooling/nargo_cli/src/cli/new_cmd.rs b/tooling/nargo_cli/src/cli/new_cmd.rs index f8a9d206cbd..db9257b8aa0 100644 --- a/tooling/nargo_cli/src/cli/new_cmd.rs +++ b/tooling/nargo_cli/src/cli/new_cmd.rs @@ -7,25 +7,25 @@ use std::path::PathBuf; /// Create a Noir project in a new directory. #[derive(Debug, Clone, Args)] -pub struct NewCommand { +pub(crate) struct NewCommand { /// The path to save the new project - pub path: PathBuf, + path: PathBuf, /// Name of the package [default: package directory name] #[clap(long)] - pub name: Option, + name: Option, /// Use a library template #[arg(long, conflicts_with = "bin", conflicts_with = "contract")] - pub lib: bool, + pub(crate) lib: bool, /// Use a binary template [default] #[arg(long, conflicts_with = "lib", conflicts_with = "contract")] - pub bin: bool, + pub(crate) bin: bool, /// Use a contract template #[arg(long, conflicts_with = "lib", conflicts_with = "bin")] - pub contract: bool, + pub(crate) contract: bool, } pub(crate) fn run(args: NewCommand, config: NargoConfig) -> Result<(), CliError> { diff --git a/tooling/nargo_cli/src/cli/test_cmd.rs b/tooling/nargo_cli/src/cli/test_cmd.rs index 13726629d82..7b0201226ef 100644 --- a/tooling/nargo_cli/src/cli/test_cmd.rs +++ b/tooling/nargo_cli/src/cli/test_cmd.rs @@ -23,32 +23,32 @@ use super::NargoConfig; /// Run the tests for this program #[derive(Debug, Clone, Args)] #[clap(visible_alias = "t")] -pub struct TestCommand { +pub(crate) struct TestCommand { /// If given, only tests with names containing this string will be run - pub test_name: Option, + test_name: Option, /// Display output of `println` statements #[arg(long)] - pub show_output: bool, + show_output: bool, /// Only run tests that match exactly #[clap(long)] - pub exact: bool, + exact: bool, /// The name of the package to test #[clap(long, conflicts_with = "workspace")] - pub package: Option, + package: Option, /// Test all packages in the workspace #[clap(long, conflicts_with = "package")] - pub workspace: bool, + workspace: bool, #[clap(flatten)] - pub compile_options: CompileOptions, + compile_options: CompileOptions, /// JSON RPC url to solve oracle calls #[clap(long)] - pub oracle_resolver: Option, + oracle_resolver: Option, } pub(crate) fn run(args: TestCommand, config: NargoConfig) -> Result<(), CliError> { diff --git a/tooling/nargo_cli/src/lib.rs b/tooling/nargo_cli/src/lib.rs deleted file mode 100644 index 5bd20108439..00000000000 --- a/tooling/nargo_cli/src/lib.rs +++ /dev/null @@ -1,13 +0,0 @@ -#![forbid(unsafe_code)] -#![warn(unreachable_pub)] -#![warn(clippy::semicolon_if_nothing_returned)] -#![cfg_attr(not(test), warn(unused_crate_dependencies, unused_extern_crates))] - -//! Nargo is the package manager for Noir -//! This name was used because it sounds like `cargo` and -//! Noir Package Manager abbreviated is npm, which is already taken. - -pub mod cli; -mod errors; - -pub const PANIC_MESSAGE: &str = "This is a bug. We may have already fixed this in newer versions of Nargo so try searching for similar issues at https://github.com/noir-lang/noir/issues/.\nIf there isn't an open issue for this bug, consider opening one at https://github.com/noir-lang/noir/issues/new?labels=bug&template=bug_report.yml"; diff --git a/tooling/nargo_cli/src/main.rs b/tooling/nargo_cli/src/main.rs index da274a27f38..a407d467ced 100644 --- a/tooling/nargo_cli/src/main.rs +++ b/tooling/nargo_cli/src/main.rs @@ -1,3 +1,15 @@ +#![forbid(unsafe_code)] +#![warn(unreachable_pub)] +#![warn(clippy::semicolon_if_nothing_returned)] +#![cfg_attr(not(test), warn(unused_crate_dependencies, unused_extern_crates))] + +//! Nargo is the package manager for Noir +//! This name was used because it sounds like `cargo` and +//! Noir Package Manager abbreviated is npm, which is already taken. + +mod cli; +mod errors; + use std::env; use color_eyre::config::HookBuilder; @@ -5,7 +17,7 @@ use color_eyre::config::HookBuilder; use tracing_appender::rolling; use tracing_subscriber::{fmt::format::FmtSpan, EnvFilter}; -use nargo_cli::{cli, PANIC_MESSAGE}; +const PANIC_MESSAGE: &str = "This is a bug. We may have already fixed this in newer versions of Nargo so try searching for similar issues at https://github.com/noir-lang/noir/issues/.\nIf there isn't an open issue for this bug, consider opening one at https://github.com/noir-lang/noir/issues/new?labels=bug&template=bug_report.yml"; fn main() { // Setup tracing From 64875223b18023b8bd1222289a3d6a5c4c103671 Mon Sep 17 00:00:00 2001 From: Akosh Farkash Date: Wed, 9 Oct 2024 13:57:30 +0100 Subject: [PATCH 4/9] Call the executor directly from the benchmark --- tooling/nargo_cli/benches/criterion.rs | 137 ++++++++++++++++++++----- 1 file changed, 112 insertions(+), 25 deletions(-) diff --git a/tooling/nargo_cli/benches/criterion.rs b/tooling/nargo_cli/benches/criterion.rs index 82cc0fe2137..538d39cea0c 100644 --- a/tooling/nargo_cli/benches/criterion.rs +++ b/tooling/nargo_cli/benches/criterion.rs @@ -1,53 +1,140 @@ //! Select representative tests to bench with criterion +use acvm::{acir::native_types::WitnessMap, FieldElement}; use assert_cmd::prelude::{CommandCargoExt, OutputAssertExt}; use criterion::{criterion_group, criterion_main, Criterion}; -use nargo_cli::cli; -use noirc_driver::CompileOptions; +use noirc_abi::{ + input_parser::{Format, InputValue}, + Abi, InputMap, +}; +use noirc_artifacts::program::ProgramArtifact; +use noirc_driver::CompiledProgram; use pprof::criterion::{Output, PProfProfiler}; +use std::path::Path; +use std::{cell::RefCell, collections::BTreeMap}; use std::{process::Command, time::Duration}; + include!("./utils.rs"); +/// Compile the test program in a sub-process +fn compile_program(test_program_dir: &Path) { + let mut cmd = Command::cargo_bin("nargo").unwrap(); + cmd.arg("--program-dir").arg(&test_program_dir); + cmd.arg("compile"); + cmd.arg("--force"); + cmd.assert().success(); +} + +/// Read the bytecode of the program from the compilation artifacts. +/// +/// Based on `ExecuteCommand::run`. +fn read_compiled_program_with_input(dir: &Path) -> (CompiledProgram, WitnessMap) { + let toml_path = nargo_toml::get_package_manifest(dir).expect("failed to read manifest"); + let workspace = nargo_toml::resolve_workspace_from_toml( + &toml_path, + nargo_toml::PackageSelection::All, + Some(noirc_driver::NOIR_ARTIFACT_VERSION_STRING.to_string()), + ) + .expect("failed to resolve workspace"); + + // Assuming there is only one binary package in these tests. + let package = workspace + .into_iter() + .filter(|package| package.is_binary()) + .last() + .expect("no binary packages in the test program"); + + let program_artifact_path = workspace.package_build_path(package); + let program: CompiledProgram = read_program_from_file(&program_artifact_path).into(); + + let (inputs, _) = read_inputs_from_file( + &package.root_dir, + nargo::constants::PROVER_INPUT_FILE, + Format::Toml, + &program.abi, + ); + + let initial_witness = + program.abi.encode(&inputs, None).expect("failed to encode input witness"); + + (program, initial_witness) +} + +/// Read the bytecode and ABI from the compilation output +fn read_program_from_file(circuit_path: &Path) -> ProgramArtifact { + let file_path = circuit_path.with_extension("json"); + let input_string = std::fs::read(&file_path).expect("failed to read artifact file"); + serde_json::from_slice(&input_string).expect("failed to deserialize artifact") +} + +/// Read the inputs from Prover.toml +fn read_inputs_from_file( + path: &Path, + file_name: &str, + format: Format, + abi: &Abi, +) -> (InputMap, Option) { + if abi.is_empty() { + return (BTreeMap::new(), None); + } + + let file_path = path.join(file_name).with_extension(format.ext()); + if !file_path.exists() { + if abi.parameters.is_empty() { + return (BTreeMap::new(), None); + } else { + panic!("input file doesn't exist: {}", file_path.display()); + } + } + + let input_string = std::fs::read_to_string(file_path).expect("failed to read input file"); + let mut input_map = format.parse(&input_string, abi).expect("failed to parse input"); + let return_value = input_map.remove(noirc_abi::MAIN_RETURN_NAME); + + (input_map, return_value) +} + /// Use the nargo CLI to compile a test program, then benchmark its execution /// by executing the command directly from the benchmark, so that we can have /// meaningful flamegraphs about the ACVM. fn criterion_selected_tests_execution(c: &mut Criterion) { for test_program_dir in get_selected_tests() { - let mut compiled = false; let benchmark_name = format!("{}_execute", test_program_dir.file_name().unwrap().to_str().unwrap()); + // The program and its inputs will be passed in the first setup. + let artifacts = RefCell::new(None); + + let mut foreign_call_executor = + nargo::ops::DefaultForeignCallExecutor::new(false, None, None, None); + c.bench_function(&benchmark_name, |b| { b.iter_batched( || { // Setup will be called many times to set a batch (which we don't use), // but we can compile it only once, and then the executions will not have to do so. // It is done as a setup so that we only compile the test programs that we filter for. - if !compiled { - compiled = true; - let mut cmd = Command::cargo_bin("nargo").unwrap(); - cmd.arg("--program-dir").arg(&test_program_dir); - cmd.arg("compile"); - cmd.arg("--force"); - cmd.assert().success(); + if artifacts.borrow().is_some() { + return; } + compile_program(&test_program_dir); + // Parse the artifacts for use in the benchmark routine + let program_and_input = read_compiled_program_with_input(&test_program_dir); + // Store them for execution + artifacts.replace(Some(program_and_input)); }, |_| { - let cmd = cli::NargoCli { - command: cli::NargoCommand::Execute(cli::execute_cmd::ExecuteCommand { - witness_name: None, - prover_name: nargo::constants::PROVER_INPUT_FILE.to_string(), - package: None, - workspace: true, - compile_options: CompileOptions { - silence_warnings: true, - ..Default::default() - }, - oracle_resolver: None, - }), - config: cli::NargoConfig { program_dir: test_program_dir.clone() }, - }; - cli::run_cmd(cmd).expect("failed to execute command"); + let artifacts = artifacts.borrow(); + let (program, initial_witness) = + artifacts.as_ref().expect("setup compiled the program"); + + let _witness_stack = nargo::ops::execute_program( + &program.program, + initial_witness.clone(), + &bn254_blackbox_solver::Bn254BlackBoxSolver, + &mut foreign_call_executor, + ) + .expect("failed to execute program"); }, criterion::BatchSize::SmallInput, ); From 1b62ac1f30dd9eb7daf034126d98abe1765202e3 Mon Sep 17 00:00:00 2001 From: Akosh Farkash Date: Wed, 9 Oct 2024 14:25:45 +0100 Subject: [PATCH 5/9] Use black_box --- tooling/nargo_cli/benches/criterion.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tooling/nargo_cli/benches/criterion.rs b/tooling/nargo_cli/benches/criterion.rs index 538d39cea0c..08d523e89d5 100644 --- a/tooling/nargo_cli/benches/criterion.rs +++ b/tooling/nargo_cli/benches/criterion.rs @@ -10,6 +10,7 @@ use noirc_abi::{ use noirc_artifacts::program::ProgramArtifact; use noirc_driver::CompiledProgram; use pprof::criterion::{Output, PProfProfiler}; +use std::hint::black_box; use std::path::Path; use std::{cell::RefCell, collections::BTreeMap}; use std::{process::Command, time::Duration}; @@ -128,12 +129,12 @@ fn criterion_selected_tests_execution(c: &mut Criterion) { let (program, initial_witness) = artifacts.as_ref().expect("setup compiled the program"); - let _witness_stack = nargo::ops::execute_program( - &program.program, - initial_witness.clone(), + let _witness_stack = black_box(nargo::ops::execute_program( + black_box(&program.program), + black_box(initial_witness.clone()), &bn254_blackbox_solver::Bn254BlackBoxSolver, &mut foreign_call_executor, - ) + )) .expect("failed to execute program"); }, criterion::BatchSize::SmallInput, From 88c6942ce775ea373bace1ce27ce657aa9896755 Mon Sep 17 00:00:00 2001 From: Akosh Farkash Date: Wed, 9 Oct 2024 14:29:01 +0100 Subject: [PATCH 6/9] Fix clippy --- tooling/nargo_cli/benches/criterion.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tooling/nargo_cli/benches/criterion.rs b/tooling/nargo_cli/benches/criterion.rs index 08d523e89d5..893ba2f2ad0 100644 --- a/tooling/nargo_cli/benches/criterion.rs +++ b/tooling/nargo_cli/benches/criterion.rs @@ -20,7 +20,7 @@ include!("./utils.rs"); /// Compile the test program in a sub-process fn compile_program(test_program_dir: &Path) { let mut cmd = Command::cargo_bin("nargo").unwrap(); - cmd.arg("--program-dir").arg(&test_program_dir); + cmd.arg("--program-dir").arg(test_program_dir); cmd.arg("compile"); cmd.arg("--force"); cmd.assert().success(); @@ -64,7 +64,7 @@ fn read_compiled_program_with_input(dir: &Path) -> (CompiledProgram, WitnessMap< /// Read the bytecode and ABI from the compilation output fn read_program_from_file(circuit_path: &Path) -> ProgramArtifact { let file_path = circuit_path.with_extension("json"); - let input_string = std::fs::read(&file_path).expect("failed to read artifact file"); + let input_string = std::fs::read(file_path).expect("failed to read artifact file"); serde_json::from_slice(&input_string).expect("failed to deserialize artifact") } From 63ae996e283a2c7f8be3762cc4fca8c9e7a21428 Mon Sep 17 00:00:00 2001 From: Akosh Farkash Date: Wed, 9 Oct 2024 18:36:00 +0100 Subject: [PATCH 7/9] Return all packages with inputs --- tooling/nargo_cli/benches/criterion.rs | 68 +++++++++++++------------- 1 file changed, 35 insertions(+), 33 deletions(-) diff --git a/tooling/nargo_cli/benches/criterion.rs b/tooling/nargo_cli/benches/criterion.rs index 893ba2f2ad0..5f2bd714ae7 100644 --- a/tooling/nargo_cli/benches/criterion.rs +++ b/tooling/nargo_cli/benches/criterion.rs @@ -26,10 +26,13 @@ fn compile_program(test_program_dir: &Path) { cmd.assert().success(); } -/// Read the bytecode of the program from the compilation artifacts. +/// Read the bytecode(s) of the program(s) from the compilation artifacts +/// from all the binary packages. Pair them up with their respective input. /// /// Based on `ExecuteCommand::run`. -fn read_compiled_program_with_input(dir: &Path) -> (CompiledProgram, WitnessMap) { +fn read_compiled_programs_and_inputs( + dir: &Path, +) -> Vec<(CompiledProgram, WitnessMap)> { let toml_path = nargo_toml::get_package_manifest(dir).expect("failed to read manifest"); let workspace = nargo_toml::resolve_workspace_from_toml( &toml_path, @@ -38,27 +41,26 @@ fn read_compiled_program_with_input(dir: &Path) -> (CompiledProgram, WitnessMap< ) .expect("failed to resolve workspace"); - // Assuming there is only one binary package in these tests. - let package = workspace - .into_iter() - .filter(|package| package.is_binary()) - .last() - .expect("no binary packages in the test program"); + let mut programs = Vec::new(); + let binary_packages = workspace.into_iter().filter(|package| package.is_binary()); - let program_artifact_path = workspace.package_build_path(package); - let program: CompiledProgram = read_program_from_file(&program_artifact_path).into(); + for package in binary_packages { + let program_artifact_path = workspace.package_build_path(package); + let program: CompiledProgram = read_program_from_file(&program_artifact_path).into(); - let (inputs, _) = read_inputs_from_file( - &package.root_dir, - nargo::constants::PROVER_INPUT_FILE, - Format::Toml, - &program.abi, - ); + let (inputs, _) = read_inputs_from_file( + &package.root_dir, + nargo::constants::PROVER_INPUT_FILE, + Format::Toml, + &program.abi, + ); - let initial_witness = - program.abi.encode(&inputs, None).expect("failed to encode input witness"); + let initial_witness = + program.abi.encode(&inputs, None).expect("failed to encode input witness"); - (program, initial_witness) + programs.push((program, initial_witness)); + } + programs } /// Read the bytecode and ABI from the compilation output @@ -104,7 +106,7 @@ fn criterion_selected_tests_execution(c: &mut Criterion) { format!("{}_execute", test_program_dir.file_name().unwrap().to_str().unwrap()); // The program and its inputs will be passed in the first setup. - let artifacts = RefCell::new(None); + let artifacts = RefCell::new(Vec::new()); let mut foreign_call_executor = nargo::ops::DefaultForeignCallExecutor::new(false, None, None, None); @@ -115,27 +117,27 @@ fn criterion_selected_tests_execution(c: &mut Criterion) { // Setup will be called many times to set a batch (which we don't use), // but we can compile it only once, and then the executions will not have to do so. // It is done as a setup so that we only compile the test programs that we filter for. - if artifacts.borrow().is_some() { + if !artifacts.borrow().is_empty() { return; } compile_program(&test_program_dir); // Parse the artifacts for use in the benchmark routine - let program_and_input = read_compiled_program_with_input(&test_program_dir); + let programs = read_compiled_programs_and_inputs(&test_program_dir); // Store them for execution - artifacts.replace(Some(program_and_input)); + artifacts.replace(programs); }, |_| { let artifacts = artifacts.borrow(); - let (program, initial_witness) = - artifacts.as_ref().expect("setup compiled the program"); - - let _witness_stack = black_box(nargo::ops::execute_program( - black_box(&program.program), - black_box(initial_witness.clone()), - &bn254_blackbox_solver::Bn254BlackBoxSolver, - &mut foreign_call_executor, - )) - .expect("failed to execute program"); + + for (program, initial_witness) in artifacts.iter() { + let _witness_stack = black_box(nargo::ops::execute_program( + black_box(&program.program), + black_box(initial_witness.clone()), + &bn254_blackbox_solver::Bn254BlackBoxSolver, + &mut foreign_call_executor, + )) + .expect("failed to execute program"); + } }, criterion::BatchSize::SmallInput, ); From 5e60e3134934c7a64bafb90e8f2212f4f859b136 Mon Sep 17 00:00:00 2001 From: Akosh Farkash Date: Wed, 9 Oct 2024 18:47:41 +0100 Subject: [PATCH 8/9] Collect all benchmarks as selected tests --- tooling/nargo_cli/benches/utils.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/tooling/nargo_cli/benches/utils.rs b/tooling/nargo_cli/benches/utils.rs index 47968f7e898..37c94241b08 100644 --- a/tooling/nargo_cli/benches/utils.rs +++ b/tooling/nargo_cli/benches/utils.rs @@ -6,6 +6,7 @@ fn get_selected_tests() -> Vec { Ok(dir) => PathBuf::from(dir), Err(_) => std::env::current_dir().unwrap(), }; + let test_dir = manifest_dir .parent() .unwrap() @@ -15,5 +16,11 @@ fn get_selected_tests() -> Vec { .join("execution_success"); let selected_tests = vec!["struct", "eddsa", "regression"]; - selected_tests.into_iter().map(|t| test_dir.join(t)).collect() + let mut selected_tests = + selected_tests.into_iter().map(|t| test_dir.join(t)).collect::>(); + + let test_dir = test_dir.parent().unwrap().join("benchmarks"); + selected_tests.extend(test_dir.read_dir().unwrap().filter_map(|e| e.ok()).map(|e| e.path())); + + selected_tests } From 86f1fc9237979135620faa4dccd4b65d6c368f40 Mon Sep 17 00:00:00 2001 From: Akosh Farkash Date: Wed, 9 Oct 2024 19:38:32 +0100 Subject: [PATCH 9/9] Warn if there are no packages to benchmark --- tooling/nargo_cli/benches/criterion.rs | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/tooling/nargo_cli/benches/criterion.rs b/tooling/nargo_cli/benches/criterion.rs index 5f2bd714ae7..949f7add393 100644 --- a/tooling/nargo_cli/benches/criterion.rs +++ b/tooling/nargo_cli/benches/criterion.rs @@ -105,8 +105,8 @@ fn criterion_selected_tests_execution(c: &mut Criterion) { let benchmark_name = format!("{}_execute", test_program_dir.file_name().unwrap().to_str().unwrap()); - // The program and its inputs will be passed in the first setup. - let artifacts = RefCell::new(Vec::new()); + // The program and its inputs will be populated in the first setup. + let artifacts = RefCell::new(None); let mut foreign_call_executor = nargo::ops::DefaultForeignCallExecutor::new(false, None, None, None); @@ -117,19 +117,24 @@ fn criterion_selected_tests_execution(c: &mut Criterion) { // Setup will be called many times to set a batch (which we don't use), // but we can compile it only once, and then the executions will not have to do so. // It is done as a setup so that we only compile the test programs that we filter for. - if !artifacts.borrow().is_empty() { + if artifacts.borrow().is_some() { return; } compile_program(&test_program_dir); // Parse the artifacts for use in the benchmark routine let programs = read_compiled_programs_and_inputs(&test_program_dir); + // Warn, but don't stop, if we haven't found any binary packages. + if programs.is_empty() { + eprintln!("\nWARNING: There is nothing to benchmark in {benchmark_name}"); + } // Store them for execution - artifacts.replace(programs); + artifacts.replace(Some(programs)); }, |_| { let artifacts = artifacts.borrow(); + let artifacts = artifacts.as_ref().expect("setup compiled them"); - for (program, initial_witness) in artifacts.iter() { + for (program, initial_witness) in artifacts { let _witness_stack = black_box(nargo::ops::execute_program( black_box(&program.program), black_box(initial_witness.clone()),