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): Generate circuit benchmark summaries on PRs #3250

Merged
merged 7 commits into from
Oct 23, 2023
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
90 changes: 90 additions & 0 deletions .github/workflows/gates_report.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
name: Report gates 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/rust-toolchain@1.66.0

- 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@v3
with:
name: nargo
path: ./dist/*
retention-days: 3


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

steps:
- uses: actions/checkout@v3
with:
submodules: recursive

- name: Download nargo binary
uses: actions/download-artifact@v3
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 gates report
working-directory: ./tooling/nargo_cli/tests
run: |
./gates_report.sh
mv gates_report.json ../../../gates_report.json

- name: Compare gates reports
id: gates_diff
uses: TomAFrench/noir-gates-diff@e7cf131b7e7f044c01615f93f0b855f65ddc02d4
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can change this in a followup PR

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, we need to do some iterations before we can make a proper releases of the action.

with:
report: gates_report.json
summaryQuantile: 0.9 # only display the 10% most significant circuit size diffs in the summary (defaults to 20%)

- name: Add gates diff to sticky comment
if: github.event_name == 'pull_request' || github.event_name == 'pull_request_target'
uses: marocchino/sticky-pull-request-comment@v2
with:
# delete the comment in case changes no longer impact circuit sizes
delete: ${{ !steps.gates_diff.outputs.markdown }}
message: ${{ steps.gates_diff.outputs.markdown }}
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@ result
!tooling/nargo_cli/tests/acir_artifacts/*/target
!tooling/nargo_cli/tests/acir_artifacts/*/target/witness.gz
!compiler/wasm/noir-script/target

gates_report.json

# Github Actions scratch space
# This gives a location to download artifacts into the repository in CI without making git dirty.
libbarretenberg-wasm32
Expand Down
20 changes: 4 additions & 16 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions cspell.json
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@
"prettytable",
"printstd",
"pseudocode",
"quantile",
"rustc",
"rustup",
"schnorr",
Expand Down
2 changes: 1 addition & 1 deletion tooling/nargo_cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ toml.workspace = true
serde.workspace = true
serde_json.workspace = true
prettytable-rs = "0.10"
rayon = "1.7.0"
rayon = "1.8.0"
thiserror.workspace = true
tower.workspace = true
async-lsp = { version = "0.0.5", default-features = false, features = [
Expand Down
36 changes: 36 additions & 0 deletions tooling/nargo_cli/tests/gates_report.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
#!/bin/bash
set -e

# These tests are incompatible with gas reporting
excluded_dirs=("workspace" "workspace_default_member")

# These tests cause failures in CI with a stack overflow for some reason.
ci_excluded_dirs=("eddsa")

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 --json > gates_report.json

rm Nargo.toml
Loading