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

Add tracking for compilation times of important circuits in CI #6722

Closed
TomAFrench opened this issue Dec 6, 2024 · 1 comment · Fixed by #6750
Closed

Add tracking for compilation times of important circuits in CI #6722

TomAFrench opened this issue Dec 6, 2024 · 1 comment · Fixed by #6750
Assignees

Comments

@TomAFrench
Copy link
Member

We currently have no good way to quantitatively track whether a PR will cause a regression in compilation time for important user circuits. A recent example of this coming back to bite us is in the bloblib circuits where compilation times jumped from 10 to 90mins.

Similar to run-external-checks, we should add a workflow to CI which will compile a number of protocol circuits (the rollup circuits in particular) and measure the time taken to do so. This can then:

  • Print a comment to the PR showing the time and highlight any regressions (would require a modification of https://github.com/noir-lang/noir-bench-report to support this new metric)
  • Define a hard limit on compilation times which will automatically block the PR if exceeded.

As a first cut we can just use time nargo compile --force to extract the compilation time so this is just a devops issue and shouldn't need modifying the compiler.

@TomAFrench
Copy link
Member Author

If we're compiling many protocol circuits as part of CI we may want to split this over multiple runners and then merge the reports together after the fact. This can be done later though to avoid premature optimization.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: ✅ Done
Development

Successfully merging a pull request may close this issue.

2 participants