-
Notifications
You must be signed in to change notification settings - Fork 738
(ci): Added benchmarking to nightly #9631
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
base: eytan_graphite/_feat_using_criterion_benchmarking_framework_with_simple_compile_benchmark
Are you sure you want to change the base?
Conversation
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7a3206245e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
.github/workflows/nightly.yml
Outdated
| - uses: Swatinem/rust-cache@v2 | ||
|
|
||
| - name: Run benchmarks | ||
| run: cargo bench -- --output-format bencher 2>&1 | tee benchmark-output.txt |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Preserve failure status when capturing benchmark output
The Run benchmarks command pipes cargo bench into tee, so the pipeline can succeed even when cargo bench itself fails because the step reports the last command’s status. In this workflow that means a broken benchmark run can still proceed to Store benchmark results and git push, which risks publishing misleading nightly benchmark data; add set -o pipefail (or avoid the pipe) so benchmark failures fail the job.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
c5c34f2 to
ac232c9
Compare
7a32062 to
2a08b9f
Compare
eytan-starkware
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@eytan-starkware made 1 comment.
Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @orizi and @TomerStarkware).
.github/workflows/nightly.yml
Outdated
| - uses: Swatinem/rust-cache@v2 | ||
|
|
||
| - name: Run benchmarks | ||
| run: cargo bench -- --output-format bencher 2>&1 | tee benchmark-output.txt |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
eytan-starkware
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@eytan-starkware resolved 1 discussion.
Reviewable status: 0 of 1 files reviewed, all discussions resolved (waiting on @orizi and @TomerStarkware).
orizi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@orizi reviewed all commit messages and made 1 comment.
Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @eytan-starkware and @TomerStarkware).
.github/workflows/nightly.yml line 76 at r2 (raw file):
run: | set -o pipefail cargo bench -- --output-format bencher 2>&1 | tee benchmark-output.txt
probably required.
Suggestion:
cargo bench --profile=release -- --output-format bencher 2>&1 | tee benchmark-output.txt
eytan-starkware
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@eytan-starkware made 1 comment.
Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @orizi and @TomerStarkware).
.github/workflows/nightly.yml line 76 at r2 (raw file):
Previously, orizi wrote…
probably required.
```By default, cargo bench uses the `bench` profile, which enables optimizations and disables debugging information
bench
The bench profile is the default profile used by cargo bench. The bench profile inherits the settings from the release profile.
orizi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@orizi reviewed 1 file, made 1 comment, and resolved 1 discussion.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @TomerStarkware).

Summary
Added a new benchmark job to the nightly workflow that runs cargo benchmarks and stores the results. The benchmark job:
cargo benchand captures outputThis enables tracking compiler performance over time with automatic benchmarking.
Type of change
Please check one:
Why is this change needed?
This PR adds automated benchmarking to track compiler performance metrics over time. By running benchmarks nightly and storing the results, we can identify performance regressions and improvements, making it easier to maintain and optimize the Cairo compiler.
What was the behavior or documentation before?
Previously, there was no automated benchmarking in the nightly workflow, making it difficult to track performance changes over time.
What is the behavior or documentation after?
The nightly workflow now includes a benchmark job that:
cargo benchAdditional context
The benchmark results will be available on GitHub Pages, providing a historical view of compiler performance metrics. This helps maintainers identify performance trends and potential issues before they impact users.