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

chore(ci)!: Add Brillig loop bytecode size regression and update noir-gates-diff report #5747

Merged
merged 15 commits into from
Aug 19, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 1 addition & 1 deletion .github/workflows/gates_report.yml
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ jobs:

- name: Compare gates reports
id: gates_diff
uses: vezenovm/noir-gates-diff@acf12797860f237117e15c0d6e08d64253af52b6
uses: noir-lang/noir-gates-diff@1931aaaa848a1a009363d6115293f7b7fc72bb87
with:
report: gates_report.json
summaryQuantile: 0.9 # only display the 10% most significant circuit size diffs in the summary (defaults to 20%)
Expand Down
92 changes: 92 additions & 0 deletions .github/workflows/gates_report_brillig.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
name: Report Brillig bytecode size 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_bytecode_size_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 bytecode size report
working-directory: ./test_programs
run: |
chmod +x gates_report_brillig.sh
./gates_report_brillig.sh
mv gates_report_brillig.json ../gates_report_brillig.json

- name: Compare Brillig bytecode size reports
id: brillig_bytecode_diff
uses: noir-lang/noir-gates-diff@3fb844067b25d1b59727ea600b614503b33503f4
with:
report: gates_report_brillig.json
header: |
# Changes to Brillig bytecode sizes
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
# delete the comment in case changes no longer impact brillig bytecode sizes
delete: ${{ !steps.brillig_bytecode_diff.outputs.markdown }}
message: ${{ steps.brillig_bytecode_diff.outputs.markdown }}
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ tooling/noir_js/lib
!compiler/wasm/noir-script/target

gates_report.json
gates_report_brillig.json

# Github Actions scratch space
# This gives a location to download artifacts into the repository in CI without making git dirty.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "brillig_loop_size_regression"
type = "bin"
authors = [""]
compiler_version = ">=0.33.0"

[dependencies]
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
struct EnumEmulation {
a: Option<Field>,
b: Option<Field>,
c: Option<Field>,
}

unconstrained fn main() -> pub Field {
let mut emulated_enum = EnumEmulation { a: Option::some(1), b: Option::none(), c: Option::none() };

for _ in 0..1 {
assert_eq(emulated_enum.a.unwrap(), 1);
}

emulated_enum.a = Option::some(2);
emulated_enum.a.unwrap()
}
33 changes: 33 additions & 0 deletions test_programs/gates_report_brillig.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
#!/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")

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 --force-brillig --json > gates_report_brillig.json

rm Nargo.toml
2 changes: 1 addition & 1 deletion tooling/nargo_cli/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ fn generate_compile_success_empty_tests(test_file: &mut File, test_data_dir: &Pa
let json: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap_or_else(|e| {{
panic!("JSON was not well-formatted {:?}\n\n{:?}", e, std::str::from_utf8(&output.stdout))
}});
let num_opcodes = &json["programs"][0]["functions"][0]["acir_opcodes"];
let num_opcodes = &json["programs"][0]["functions"][0]["opcodes"];
assert_eq!(num_opcodes.as_u64().expect("number of opcodes should fit in a u64"), 0);
"#;

Expand Down
13 changes: 7 additions & 6 deletions tooling/nargo_cli/src/cli/info_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,7 @@ struct ProgramInfo {
#[serde(skip)]
expression_width: ExpressionWidth,
functions: Vec<FunctionInfo>,
#[serde(skip)]
unconstrained_functions_opcodes: usize,
unconstrained_functions: Vec<FunctionInfo>,
}
Expand All @@ -186,7 +187,7 @@ impl From<ProgramInfo> for Vec<Row> {
Fm->format!("{}", program_info.package_name),
Fc->format!("{}", function.name),
format!("{:?}", program_info.expression_width),
Fc->format!("{}", function.acir_opcodes),
Fc->format!("{}", function.opcodes),
Fc->format!("{}", program_info.unconstrained_functions_opcodes),
]
});
Expand All @@ -196,7 +197,7 @@ impl From<ProgramInfo> for Vec<Row> {
Fc->format!("{}", function.name),
format!("N/A", ),
Fc->format!("N/A"),
Fc->format!("{}", function.acir_opcodes),
Fc->format!("{}", function.opcodes),
]
}));
main
Expand All @@ -215,7 +216,7 @@ struct ContractInfo {
#[derive(Debug, Serialize)]
struct FunctionInfo {
name: String,
acir_opcodes: usize,
opcodes: usize,
}

impl From<ContractInfo> for Vec<Row> {
Expand All @@ -225,7 +226,7 @@ impl From<ContractInfo> for Vec<Row> {
Fm->format!("{}", contract_info.name),
Fc->format!("{}", function.name),
format!("{:?}", contract_info.expression_width),
Fc->format!("{}", function.acir_opcodes),
Fc->format!("{}", function.opcodes),
]
})
}
Expand All @@ -243,7 +244,7 @@ fn count_opcodes_and_gates_in_program(
.enumerate()
.map(|(i, function)| FunctionInfo {
name: compiled_program.names[i].clone(),
acir_opcodes: function.opcodes.len(),
opcodes: function.opcodes.len(),
})
.collect();

Expand All @@ -264,7 +265,7 @@ fn count_opcodes_and_gates_in_program(
.clone()
.iter()
.zip(opcodes_len)
.map(|(name, len)| FunctionInfo { name: name.clone(), acir_opcodes: len })
.map(|(name, len)| FunctionInfo { name: name.clone(), opcodes: len })
.collect();

ProgramInfo {
Expand Down
Loading