Skip to content

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

Merged
merged 19 commits into from
Apr 25, 2025
Merged

Perf test framework #2522

merged 19 commits into from
Apr 25, 2025

Conversation

gearama
Copy link
Member

@gearama gearama commented Apr 24, 2025

Add a test with some params, add perf-tests.yml in order to get it to be executed by the perf automation.

@Copilot Copilot AI review requested due to automatic review settings April 24, 2025 23:06
@github-actions github-actions bot added the Azure.Core The azure_core crate label Apr 24, 2025
Copy link
Contributor

@Copilot Copilot AI left a 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.
      -

@azure-sdk
Copy link
Collaborator

API change check

API changes are not detected in this pull request.

Copy link
Member

@heaths heaths left a 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.

Copy link
Member

@heaths heaths left a 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.

gearama and others added 2 commits April 25, 2025 15:23
@gearama gearama requested a review from heaths April 25, 2025 22:30
@gearama gearama requested a review from hallipr as a code owner April 25, 2025 22:42
@gearama gearama requested a review from weshaggard as a code owner April 25, 2025 22:42
@gearama gearama merged commit 747a7e9 into Azure:main Apr 25, 2025
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Azure.Core The azure_core crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants