Skip to content

Commit

Permalink
chore: reactivate gates report (#5084)
Browse files Browse the repository at this point in the history
# Description

## Problem\*

Resolves <!-- Link to GitHub Issue -->

## Summary\*

This PR reactivates the gates report workflow now that we don't have
access to the number of constraints through `nargo info`

## Additional Context



## Documentation\*

Check one:
- [x] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[For Experimental Features]** Documentation to be submitted in a
separate PR.

# PR Checklist\*

- [x] I have tested the changes locally.
- [x] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.
  • Loading branch information
TomAFrench authored May 22, 2024
1 parent e807f0d commit 23a6477
Show file tree
Hide file tree
Showing 11 changed files with 113 additions and 106 deletions.
180 changes: 93 additions & 87 deletions .github/workflows/gates_report.yml
Original file line number Diff line number Diff line change
@@ -1,88 +1,94 @@
# 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.74.1

# - 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_gas_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 gates report
# working-directory: ./test_programs
# run: |
# ./gates_report.sh
# mv gates_report.json ../gates_report.json
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.74.1

- 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_gates_reports:
needs: [build-nargo]
runs-on: ubuntu-latest
permissions:
pull-requests: write

steps:
- uses: actions/checkout@v4

- name: Install `bb`
run: |
./scripts/install_bb.sh
echo "$HOME/.bb/" >> $GITHUB_PATH
- 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 gates report
working-directory: ./test_programs
run: |
./rebuild.sh
./gates_report.sh
mv gates_report.json ../gates_report.json
# - name: Compare gates reports
# id: gates_diff
# uses: vezenovm/noir-gates-diff@acf12797860f237117e15c0d6e08d64253af52b6
# 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 }}
- name: Compare gates reports
id: gates_diff
uses: vezenovm/noir-gates-diff@acf12797860f237117e15c0d6e08d64253af52b6
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 }}
39 changes: 20 additions & 19 deletions test_programs/gates_report.sh
Original file line number Diff line number Diff line change
@@ -1,36 +1,37 @@
#!/usr/bin/env bash
set -e

BACKEND=${BACKEND:-bb}

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

# 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)
artifacts_path="$current_dir/acir_artifacts"
test_dirs=$(ls $artifacts_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 "{\"programs\": [" > gates_report.json

echo "[workspace]" > Nargo.toml
echo "members = [" >> Nargo.toml
# Bound for checking where to place last parentheses
NUM_ARTIFACTS=$(ls -1q "$artifacts_path" | wc -l)

for dir in $test_dirs; do
if [[ " ${excluded_dirs[@]} " =~ " ${dir} " ]]; then
continue
fi
ITER="1"
for pathname in $test_dirs; do
ARTIFACT_NAME=$(basename "$pathname")

GATES_INFO=$($BACKEND gates -b "$artifacts_path/$ARTIFACT_NAME/target/program.json")
MAIN_FUNCTION_INFO=$(echo $GATES_INFO | jq -r '.functions[0] | .name = "main"')
echo "{\"package_name\": \"$ARTIFACT_NAME\", \"functions\": [$MAIN_FUNCTION_INFO]" >> gates_report.json

if [[ ${CI-false} = "true" ]] && [[ " ${ci_excluded_dirs[@]} " =~ " ${dir} " ]]; then
continue
if (($ITER == $NUM_ARTIFACTS)); then
echo "}" >> gates_report.json
else
echo "}, " >> gates_report.json
fi

echo " \"execution_success/$dir\"," >> Nargo.toml
ITER=$(( $ITER + 1 ))
done

echo "]" >> Nargo.toml
echo "]}" >> gates_report.json

nargo info --json > gates_report.json

rm Nargo.toml

0 comments on commit 23a6477

Please sign in to comment.