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

Conversation

TomAFrench
Copy link
Member

Description

Problem*

Resolves #3182

Summary*

This PR adds benchmarking for how PRs affect the number of opcodes/constraints generated for the programs in the execution_success test suite. If a PR changes the ACIR generated then a sticky comment will be added to the PR with a summary of the changes.

Documentation

  • This PR requires documentation updates when merged.

    • I will submit a noir-lang/docs PR.
    • I will request for and support Dev Rel's help in documenting this PR.

Additional Context

PR Checklist*

  • I have tested the changes locally.
  • I have formatted the changes with Prettier and/or cargo fmt on default settings.

@TomAFrench TomAFrench changed the title chore(ci): Generated circuit benchmark summaries on PRs chore(ci): Generate circuit benchmark summaries on PRs Oct 23, 2023
@TomAFrench
Copy link
Member Author

Here's an example output


Changes to circuit sizes

Generated at commit: 86cee4c3845bef99f38a493f19e40914a2a7ea71, compared to commit: eed7b9913b29686799bb1f07b54a2722b8c361e5

🧾 Summary (10% most significant diffs)

Program ACIR opcodes (+/-) % Circuit size (+/-) %
test_program -10 ✅ -22.73% -10 ✅ -0.36%

Full diff report 👇
Program ACIR opcodes (+/-) % Circuit size (+/-) %
test_program 34 (-10) -22.73% 2,778 (-10) -0.36%

@kevaundray
Copy link
Contributor

wow :D

@Savio-Sou
Copy link
Collaborator

Exciting! cc @jfecher dream coming true!

@TomAFrench
Copy link
Member Author

This may need an admin merge. Looks like the action is going to search through the entire history for a baseline benchmark artifact.

@TomAFrench
Copy link
Member Author

We can improve the action to simplify the bootstrapping experience but this should allow us to get started using it.


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

@kevaundray kevaundray merged commit 7e963e3 into master Oct 23, 2023
30 checks passed
@kevaundray kevaundray deleted the tf/circuit-size-diffs branch October 23, 2023 14:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Benchmark number of ACIR opcodes per PR
3 participants