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(ci): Add report of Brillig opcodes executed #6396

Merged
merged 6 commits into from
Oct 30, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
vezenovm marked this conversation as resolved.
Show resolved Hide resolved
# 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)
}
Loading