-
-
Notifications
You must be signed in to change notification settings - Fork 79
test(benches): add basic benchmark infrastructure #602
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: master
Are you sure you want to change the base?
Conversation
Pull Request Test Coverage Report for Build 20247651182Details
💛 - Coveralls |
55a35b4 to
d5bc0a9
Compare
d5bc0a9 to
60971aa
Compare
|
Example upload https://bencher.php.rs/perf/ext-php-rs CI upload only works once this is merged. |
|
@ptondereau I think this works as an initial implementation. We should extend this further especially before tackling any performance issues. |
ptondereau
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.
Look great! I tested it locally and am showing some good results indeed.
Some nitpicks
| name: Run PR Benchmarks | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v4 |
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.
| - uses: actions/checkout@v4 | |
| - uses: actions/checkout@v6 |
| name: Archive closed PR branch with Bencher | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v4 |
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.
| - uses: actions/checkout@v4 | |
| - uses: actions/checkout@v6 |
| name: Run PR Benchmarks | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v4 |
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.
| - uses: actions/checkout@v4 | |
| - uses: actions/checkout@v6 |
| name: benchmark_results.txt | ||
| path: ./benchmark_results.txt | ||
| - name: Upload Pull Request Event | ||
| uses: actions/upload-artifact@v4 |
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.
| uses: actions/upload-artifact@v4 | |
| uses: actions/upload-artifact@v6 |
| - name: Run Benchmarks | ||
| run: cargo bench --manifest-path ./benches/Cargo.toml > benchmark_results.txt | ||
| - name: Upload Benchmark Results | ||
| uses: actions/upload-artifact@v4 |
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.
| uses: actions/upload-artifact@v4 | |
| uses: actions/upload-artifact@v6 |
| for i in 0..n { | ||
| callback | ||
| .try_call(vec![&i]) | ||
| .expect("Failed to call function"); |
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.
Is there a more graceful way to report an error here instead of panicking for the benchmark?
Refs: #599
Description
Adds basic benchmarking infrastructure
TODO:
Checklist
Check the boxes that apply (put an
xin the brackets, like[x]). You can also check boxes after the PR is created.❤️ Thank you for your contribution!