-
Notifications
You must be signed in to change notification settings - Fork 280
Perf test framework #2522
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
Perf test framework #2522
Conversation
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.
Pull Request Overview
This PR introduces a performance test framework for the azure_core SDK by adding configuration files, benchmark code, and documentation updates.
- Added a YAML configuration file (perf-tests.yml) for performance test execution.
- Implemented a benchmark using the criterion crate in core_benchmarks.rs.
- Updated the README.md and Cargo.toml to support performance testing.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
sdk/core/azure_core/perf-tests.yml | Added YAML config for running performance tests |
sdk/core/azure_core/benches/core_benchmarks.rs | Added benchmark code using criterion for URL parsing |
sdk/core/azure_core/README.md | Updated documentation regarding performance testing |
sdk/core/azure_core/Cargo.toml | Added criterion as a dev-dependency and bench configuration |
Comments suppressed due to low confidence (1)
sdk/core/azure_core/perf-tests.yml:12
- [nitpick] The empty argument list under 'Arguments:' might be unclear; please add a comment or a placeholder value if this is intentional for clarity.
-
…into PerfTestFramework
API change check API changes are not detected in this pull request. |
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.
It's a great start, but we need to figure out what precedent to set. We can do most of that in another PR, but for this one there's a few changes I'd like to see like 1) rename the file, and 2) get rid of the grouping inside the benchmark. It's unnecessary. Grouping tests within a benchmark function is more for comparing a single benchmark sliced in different ways, like I did in my ordinal
crate I linked to. With just one benchmark, a group is just distracting and more code that partners need to needlessly write.
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.
A few nits and otherwise good to go.
Co-authored-by: Heath Stewart <[email protected]>
…sdk-for-rust into PerfTestFramework
Add a test with some params, add perf-tests.yml in order to get it to be executed by the perf automation.