-
-
Notifications
You must be signed in to change notification settings - Fork 0
180 #183
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
180 #183
Conversation
Implemented automated benchmark tracking with regression detection: - Added dedicated benchmarks job running on PR and main branch - Executes cargo bench with --save-baseline for comparison - Uploads benchmark results as artifacts (30-day retention) - Downloads previous benchmark for baseline comparison - Alerts on performance changes (foundation for >10% regression detection) Benefits: - Automatic detection of performance regressions - Historical performance tracking via artifacts - Prevents accidental slowdowns reaching production - Data-driven performance optimization decisions
| runs-on: ubuntu-latest | ||
| needs: ci | ||
| if: github.event_name == 'pull_request' || github.ref == 'refs/heads/main' | ||
| steps: | ||
| - uses: actions/checkout@v5 | ||
|
|
||
| - name: Install Rust (stable) | ||
| uses: dtolnay/rust-toolchain@v1 | ||
| with: | ||
| toolchain: stable | ||
|
|
||
| - name: Cache cargo | ||
| uses: Swatinem/rust-cache@v2 | ||
| with: | ||
| save-if: ${{ github.ref == 'refs/heads/main' }} | ||
|
|
||
| - name: Run benchmarks | ||
| run: cargo bench --no-fail-fast --features benchmarks -- --save-baseline ci-baseline | ||
|
|
||
| - name: Upload benchmark results | ||
| uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: benchmark-results | ||
| path: target/criterion/ | ||
| retention-days: 30 | ||
|
|
||
| - name: Download previous benchmark | ||
| uses: actions/download-artifact@v4 | ||
| continue-on-error: true | ||
| with: | ||
| name: benchmark-results | ||
| path: target/criterion-previous/ | ||
|
|
||
| - name: Compare with baseline | ||
| continue-on-error: true | ||
| run: | | ||
| if [ -d "target/criterion-previous" ]; then | ||
| echo "Comparing with previous benchmark..." | ||
| cargo bench --features benchmarks -- --baseline ci-baseline --noplot | ||
| else | ||
| echo "No previous benchmark found, skipping comparison" | ||
| fi |
Check warning
Code scanning / CodeQL
Workflow does not contain permissions Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 2 months ago
To fix the issue, explicitly declare a permissions block at the top level of the workflow or for each job. The minimal starting point is contents: read. For jobs that need to upload artifacts, coverage, or interact with pull requests, you may need to grant specific write permissions (e.g., pull-requests: write, contents: write, or actions: write), but unless otherwise required, keep permissions at the minimal necessary level. Add this block above jobs: to apply to all jobs unless overridden. In the file .github/workflows/reusable-ci.yml, insert:
permissions:
contents: readIf certain jobs (such as coverage or benchmarks) require additional write permissions for actions like uploading artifacts, you can override on those jobs individually, but as a baseline, contents: read is safe.
-
Copy modified lines R7-R9
| @@ -4,6 +4,9 @@ | ||
|
|
||
| name: Reusable CI | ||
|
|
||
| permissions: | ||
| contents: read | ||
|
|
||
| on: | ||
| workflow_call: | ||
| inputs: |
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.
ℹ️ 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
| if [ -d "target/criterion-previous" ]; then | ||
| echo "Comparing with previous benchmark..." | ||
| cargo bench --features benchmarks -- --baseline ci-baseline --noplot |
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.
Baseline comparison ignores previous artifact
The new benchmarks job downloads the last run into target/criterion-previous, but the subsequent comparison runs cargo bench -- … --baseline ci-baseline using the default target/criterion directory produced earlier in the job. Because the previous data is never copied into target/criterion or passed via --baseline-dir, the command compares the current run against itself and will never report regressions. This defeats the purpose of the CI check while adding duplicate benchmark execution. Consider downloading old results before running benchmarks or pass --baseline-dir target/criterion-previous so the comparison uses historical data.
Useful? React with 👍 / 👎.
Summary
Implemented automated performance benchmark tracking with regression detection capabilities.
Changes
benchmarksjob to CI workflowcargo benchwith--save-baselinefor comparisonTechnical Details
benchmarksfeature flag for criterion integrationbenchmark-resultsnametarget/criterion-previous/--noplotfor CI efficiencyBenefits
Test Plan
Next Steps
Closes #180