Skip to content

Commit

Permalink
feat(ci): Add report of Brillig opcodes executed (#6396)
Browse files Browse the repository at this point in the history
  • Loading branch information
vezenovm authored Oct 30, 2024
1 parent 8f516d7 commit e04b026
Show file tree
Hide file tree
Showing 4 changed files with 221 additions and 12 deletions.
92 changes: 92 additions & 0 deletions .github/workflows/gates_report_brillig_execution.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
name: Report Brillig opcodes executed diff

on:
push:
branches:
- master
pull_request:

jobs:
build-nargo:
runs-on: ubuntu-latest
strategy:
matrix:
target: [x86_64-unknown-linux-gnu]

steps:
- name: Checkout Noir repo
uses: actions/checkout@v4

- name: Setup toolchain
uses: dtolnay/[email protected]

- uses: Swatinem/rust-cache@v2
with:
key: ${{ matrix.target }}
cache-on-failure: true
save-if: ${{ github.event_name != 'merge_group' }}

- name: Build Nargo
run: cargo build --package nargo_cli --release

- name: Package artifacts
run: |
mkdir dist
cp ./target/release/nargo ./dist/nargo
7z a -ttar -so -an ./dist/* | 7z a -si ./nargo-x86_64-unknown-linux-gnu.tar.gz
- name: Upload artifact
uses: actions/upload-artifact@v4
with:
name: nargo
path: ./dist/*
retention-days: 3

compare_brillig_execution_reports:
needs: [build-nargo]
runs-on: ubuntu-latest
permissions:
pull-requests: write

steps:
- uses: actions/checkout@v4

- name: Download nargo binary
uses: actions/download-artifact@v4
with:
name: nargo
path: ./nargo

- name: Set nargo on PATH
run: |
nargo_binary="${{ github.workspace }}/nargo/nargo"
chmod +x $nargo_binary
echo "$(dirname $nargo_binary)" >> $GITHUB_PATH
export PATH="$PATH:$(dirname $nargo_binary)"
nargo -V
- name: Generate Brillig execution report
working-directory: ./test_programs
run: |
chmod +x gates_report_brillig_execution.sh
./gates_report_brillig_execution.sh
mv gates_report_brillig_execution.json ../gates_report_brillig_execution.json
- name: Compare Brillig execution reports
id: brillig_execution_diff
uses: noir-lang/noir-gates-diff@d88f7523b013b9edd3f31c5cfddaef87a3fe1b48
with:
report: gates_report_brillig_execution.json
header: |
# Changes to number of Brillig opcodes executed
brillig_report: true
summaryQuantile: 0.9 # only display the 10% most significant bytecode size diffs in the summary (defaults to 20%)

- name: Add bytecode size diff to sticky comment
if: github.event_name == 'pull_request' || github.event_name == 'pull_request_target'
uses: marocchino/sticky-pull-request-comment@v2
with:
header: brillig_execution
# delete the comment in case changes no longer impact brillig bytecode sizes
delete: ${{ !steps.brillig_execution_diff.outputs.markdown }}
message: ${{ steps.brillig_execution_diff.outputs.markdown }}
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ tooling/noir_js/lib

gates_report.json
gates_report_brillig.json
gates_report_brillig_execution.json

# Github Actions scratch space
# This gives a location to download artifacts into the repository in CI without making git dirty.
Expand Down
45 changes: 45 additions & 0 deletions test_programs/gates_report_brillig_execution.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
#!/usr/bin/env bash
set -e

# These tests are incompatible with gas reporting
excluded_dirs=(
"workspace"
"workspace_default_member"
"double_verify_nested_proof"
"overlapping_dep_and_mod"
"comptime_println"
# Takes a very long time to execute as large loops do not get simplified.
"regression_4709"
# bit sizes for bigint operation doesn't match up.
"bigint"
# Expected to fail as test asserts on which runtime it is in.
"is_unconstrained"
)

current_dir=$(pwd)
base_path="$current_dir/execution_success"
test_dirs=$(ls $base_path)

# We generate a Noir workspace which contains all of the test cases
# This allows us to generate a gates report using `nargo info` for all of them at once.

echo "[workspace]" > Nargo.toml
echo "members = [" >> Nargo.toml

for dir in $test_dirs; do
if [[ " ${excluded_dirs[@]} " =~ " ${dir} " ]]; then
continue
fi

if [[ ${CI-false} = "true" ]] && [[ " ${ci_excluded_dirs[@]} " =~ " ${dir} " ]]; then
continue
fi

echo " \"execution_success/$dir\"," >> Nargo.toml
done

echo "]" >> Nargo.toml

nargo info --profile-execution --json > gates_report_brillig_execution.json

rm Nargo.toml
95 changes: 83 additions & 12 deletions tooling/nargo_cli/src/cli/info_cmd.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,21 @@
use acvm::acir::circuit::ExpressionWidth;
use bn254_blackbox_solver::Bn254BlackBoxSolver;
use clap::Args;
use iter_extended::vecmap;
use nargo::package::{CrateName, Package};
use nargo::{
constants::PROVER_INPUT_FILE,
ops::DefaultForeignCallExecutor,
package::{CrateName, Package},
};
use nargo_toml::{get_package_manifest, resolve_workspace_from_toml, PackageSelection};
use noirc_abi::input_parser::Format;
use noirc_artifacts::program::ProgramArtifact;
use noirc_driver::{CompileOptions, NOIR_ARTIFACT_VERSION_STRING};
use prettytable::{row, table, Row};
use rayon::prelude::*;
use serde::Serialize;

use crate::errors::CliError;
use crate::{cli::fs::inputs::read_inputs_from_file, errors::CliError};

use super::{
compile_cmd::{compile_workspace_full, get_target_width},
Expand Down Expand Up @@ -37,11 +43,18 @@ pub(crate) struct InfoCommand {
#[clap(long, hide = true)]
json: bool,

#[clap(long)]
profile_execution: bool,

/// The name of the toml file which contains the inputs for the prover
#[clap(long, short, default_value = PROVER_INPUT_FILE)]
prover_name: String,

#[clap(flatten)]
compile_options: CompileOptions,
}

pub(crate) fn run(args: InfoCommand, config: NargoConfig) -> Result<(), CliError> {
pub(crate) fn run(mut args: InfoCommand, config: NargoConfig) -> Result<(), CliError> {
let toml_path = get_package_manifest(&config.program_dir)?;
let default_selection =
if args.workspace { PackageSelection::All } else { PackageSelection::DefaultOrAll };
Expand All @@ -52,6 +65,11 @@ pub(crate) fn run(args: InfoCommand, config: NargoConfig) -> Result<(), CliError
Some(NOIR_ARTIFACT_VERSION_STRING.to_string()),
)?;

if args.profile_execution {
// Execution profiling is only relevant with the Brillig VM
// as a constrained circuit should have totally flattened control flow (e.g. loops and if statements).
args.compile_options.force_brillig = true;
}
// Compile the full workspace in order to generate any build artifacts.
compile_workspace_full(&workspace, &args.compile_options)?;

Expand All @@ -65,15 +83,29 @@ pub(crate) fn run(args: InfoCommand, config: NargoConfig) -> Result<(), CliError
})
.collect::<Result<_, _>>()?;

let program_info = binary_packages
.into_iter()
.par_bridge()
.map(|(package, program)| {
let target_width =
get_target_width(package.expression_width, args.compile_options.expression_width);
count_opcodes_and_gates_in_program(program, &package, target_width)
})
.collect();
let program_info = if args.profile_execution {
assert!(
args.compile_options.force_brillig,
"Internal CLI Error: --force-brillig must be active when --profile-execution is active"
);
profile_brillig_execution(
binary_packages,
&args.prover_name,
args.compile_options.expression_width,
)?
} else {
binary_packages
.into_iter()
.par_bridge()
.map(|(package, program)| {
let target_width = get_target_width(
package.expression_width,
args.compile_options.expression_width,
);
count_opcodes_and_gates_in_program(program, &package, target_width)
})
.collect()
};

let info_report = InfoReport { programs: program_info };

Expand Down Expand Up @@ -210,3 +242,42 @@ fn count_opcodes_and_gates_in_program(
unconstrained_functions: unconstrained_info,
}
}

fn profile_brillig_execution(
binary_packages: Vec<(Package, ProgramArtifact)>,
prover_name: &str,
expression_width: Option<ExpressionWidth>,
) -> Result<Vec<ProgramInfo>, CliError> {
let mut program_info = Vec::new();
for (package, program_artifact) in binary_packages.iter() {
// Parse the initial witness values from Prover.toml
let (inputs_map, _) = read_inputs_from_file(
&package.root_dir,
prover_name,
Format::Toml,
&program_artifact.abi,
)?;
let initial_witness = program_artifact.abi.encode(&inputs_map, None)?;

let (_, profiling_samples) = nargo::ops::execute_program_with_profiling(
&program_artifact.bytecode,
initial_witness,
&Bn254BlackBoxSolver,
&mut DefaultForeignCallExecutor::new(false, None, None, None),
)?;

let expression_width = get_target_width(package.expression_width, expression_width);

program_info.push(ProgramInfo {
package_name: package.name.to_string(),
expression_width,
functions: vec![FunctionInfo { name: "main".to_string(), opcodes: 0 }],
unconstrained_functions_opcodes: profiling_samples.len(),
unconstrained_functions: vec![FunctionInfo {
name: "main".to_string(),
opcodes: profiling_samples.len(),
}],
});
}
Ok(program_info)
}

0 comments on commit e04b026

Please sign in to comment.