Skip to content

Introduce new performance benchmark runners#4447

Open
cheenamalhotra wants to merge 7 commits into
mainfrom
dev/cheena/perf-project
Open

Introduce new performance benchmark runners#4447
cheenamalhotra wants to merge 7 commits into
mainfrom
dev/cheena/perf-project

Conversation

@cheenamalhotra

@cheenamalhotra cheenamalhotra commented Jul 15, 2026

Copy link
Copy Markdown
Member

Summary

Adds 8 BenchmarkDotNet runners to Microsoft.Data.SqlClient.PerformanceTests covering previously-reported hot spots, plus config/wiring updates to support them.

New runners

Runner Purpose Issues
AsyncLargeDataReadRunner.cs Sync vs async reads of large VARBINARY(MAX) (1–20 MB) #593, #1562
MarsOverheadRunner.cs Query execution with MARS on/off #422, #1283
ParallelAsyncConnectionRunner.cs Concurrent OpenAsync (10/50/100), pooling on/off #601, #979
CancellationTokenReadAsyncRunner.cs CancellationToken overhead in ReadAsync #2408
SequentialXmlReadRunner.cs SequentialAccess XML reads at increasing sizes (O(N²) check) #1877
JsonVsVarcharReadRunner.cs JSON vs VARCHAR(MAX) reads (disabled by default, needs SQL 2025+) #3499
BeginTransactionRunner.cs BeginTransaction/Async latency vs baseline #1554
ConnectionPoolStressRunner.cs Pool stress: churn, random hold, mixed sync/async, reuse, exhaustion/recovery (Parallelism {10,20,25} × MaxPoolSize {50,100}) #601, #979, #3356

Supporting changes

  • Program.cs — registers new runners, gated by RunnerJob.Enabled
  • Config/Config.cs — new config fields + RunnerJob per runner
  • Config/BenchmarkConfig.cs — DontOverwriteResults, JoinSummary, COMPlus_gcServer=1
  • runnerconfig.jsonc — entries added for new runners (JSON runner off by default), and new configuration options.

Validation

Ran all new runners locally (.NET 9.0 Release, Apple M3 Pro, Docker azure-sql-edge) — all completed successfully.

Checklist

  • Tests added (perf runners)
  • No public API changes
  • Verified against SQL Server
  • No breaking changes

Copilot AI review requested due to automatic review settings July 15, 2026 07:14
@cheenamalhotra
cheenamalhotra requested a review from a team as a code owner July 15, 2026 07:14
@github-project-automation github-project-automation Bot moved this to To triage in SqlClient Board Jul 15, 2026
@cheenamalhotra cheenamalhotra added Performance 📈 Issues that are targeted to performance improvements. Area\Tests Issues that are targeted to tests or test projects labels Jul 15, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds new BenchmarkDotNet runners to the Microsoft.Data.SqlClient.PerformanceTests harness to measure previously reported performance hot spots (large async reads, MARS overhead, parallel OpenAsync, cancellation token overhead, sequential XML reads, JSON vs VARCHAR reads, transaction latency, and connection pool stress), along with configuration and wiring updates to run them via runnerconfig.json.

Changes:

  • Registers 8 new benchmark runners and gates each by a per-runner RunnerJob.Enabled config.
  • Extends the perf test config model (Config) and default runnerconfig.json with new runner entries plus new top-level flags.
  • Updates BenchmarkDotNet ManualConfig options/environment for more consistent perf runs (e.g., dont overwrite results, joined summary, server GC).

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
src/Microsoft.Data.SqlClient/tests/PerformanceTests/runnerconfig.json Adds new top-level flags and runner job entries; enables new runners by default and disables older ones.
src/Microsoft.Data.SqlClient/tests/PerformanceTests/Program.cs Loads config, applies AppContext switches / optional profiler wait, and invokes each runner when enabled.
src/Microsoft.Data.SqlClient/tests/PerformanceTests/Config/Config.cs Adds new config fields and per-runner RunnerJob entries.
src/Microsoft.Data.SqlClient/tests/PerformanceTests/Config/BenchmarkConfig.cs Updates BDN config options and sets server GC env var; joins summaries.
src/Microsoft.Data.SqlClient/tests/PerformanceTests/BenchmarkRunners/AsyncLargeDataReadRunner.cs New runner to compare sync vs async large VARBINARY streaming reads.
src/Microsoft.Data.SqlClient/tests/PerformanceTests/BenchmarkRunners/MarsOverheadRunner.cs New runner comparing query execution with MARS enabled/disabled (sync + async).
src/Microsoft.Data.SqlClient/tests/PerformanceTests/BenchmarkRunners/ParallelAsyncConnectionRunner.cs New runner opening many connections concurrently with pooling on/off.
src/Microsoft.Data.SqlClient/tests/PerformanceTests/BenchmarkRunners/CancellationTokenReadAsyncRunner.cs New runner measuring ReadAsync iteration overhead with/without CancellationToken.
src/Microsoft.Data.SqlClient/tests/PerformanceTests/BenchmarkRunners/SequentialXmlReadRunner.cs New runner targeting sequential XML read scaling across payload sizes.
src/Microsoft.Data.SqlClient/tests/PerformanceTests/BenchmarkRunners/JsonVsVarcharReadRunner.cs New runner comparing JSON vs VARCHAR read performance (disabled by default).
src/Microsoft.Data.SqlClient/tests/PerformanceTests/BenchmarkRunners/BeginTransactionRunner.cs New runner comparing transaction vs no-transaction latency (sync + async).
src/Microsoft.Data.SqlClient/tests/PerformanceTests/BenchmarkRunners/ConnectionPoolStressRunner.cs New stress runner for pooled open/close churn, mixed sync/async contention, and exhaustion/recovery.

@apoorvdeshmukh apoorvdeshmukh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These benchmarks will be very helpful!
Wondering if it would be possible to establish a baseline for native memory consumption when running these benchmarks and monitor memory growth if it drifts too much from the established baseline? This may be beyond the current scope of the PR but worth looking into.

Copilot AI review requested due to automatic review settings July 15, 2026 07:33

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.

Comment thread BUILDGUIDE.md Outdated
Copilot AI review requested due to automatic review settings July 15, 2026 08:01
@cheenamalhotra cheenamalhotra added this to the 7.1.0-preview3 milestone Jul 15, 2026
@cheenamalhotra

cheenamalhotra commented Jul 15, 2026

Copy link
Copy Markdown
Member Author

These benchmarks will be very helpful! Wondering if it would be possible to establish a baseline for native memory consumption when running these benchmarks and monitor memory growth if it drifts too much from the established baseline? This may be beyond the current scope of the PR but worth looking into.

Added optionally, could you try and confirm as well?
(since I'm testing on mac today, I'll test this later on Windows)

@cheenamalhotra cheenamalhotra moved this from To triage to In review in SqlClient Board Jul 15, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 5 comments.

Copilot AI review requested due to automatic review settings July 15, 2026 08:13

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 8 comments.

Comment thread src/Microsoft.Data.SqlClient/tests/PerformanceTests/runnerconfig.jsonc Outdated
Comment thread BUILDGUIDE.md Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 15, 2026 17:07

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.

mdaigle
mdaigle previously approved these changes Jul 15, 2026
using var conn = new SqlConnection(_connectionString);
conn.Open();

using var createCmd = new SqlCommand(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should be able to use the RAII types here. Can be added in the future.

@cheenamalhotra cheenamalhotra Jul 15, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this is just bringing over test cases to public repo that were missing, to give a kickstart to unix async perf work. All goodness is welcome to enhance the test suite :)

Just don't change too much on configuration front as I will be setting up pipelines soon for running benchmarks internally :)

@mdaigle

mdaigle commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

I'm in favor of merging these to give us easy access to the existing tests that were previously locked away in ADO. We can improve these and add on to them as we go.

@github-project-automation github-project-automation Bot moved this from In review to Waiting for customer in SqlClient Board Jul 15, 2026
@codecov

codecov Bot commented Jul 15, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 63.95%. Comparing base (fdebcd2) to head (d67bf7a).
⚠️ Report is 12 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4447      +/-   ##
==========================================
- Coverage   65.83%   63.95%   -1.89%     
==========================================
  Files         287      284       -3     
  Lines       43763    66786   +23023     
==========================================
+ Hits        28812    42712   +13900     
- Misses      14951    24074    +9123     
Flag Coverage Δ
CI-SqlClient ?
PR-SqlClient-Project 63.95% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

- AsyncLargeDataReadRunner: seed VARBINARY(MAX) row via CRYPT_GEN_RANDOM T-SQL instead of allocating a client-side byte[] and shipping it over the wire, and add [Params] for ReadBufferBytes so we can observe how the client read buffer size interacts with payload size.
- CancellationTokenReadAsyncRunner: move CancellationTokenSource allocation to [IterationSetup] and disposal to [IterationCleanup] so only the per-row ReadAsync(CancellationToken) overhead is measured.
- ConnectionPoolStressRunner: scale the RapidFireOpenClose per-task iteration count with MaxPoolSize/Parallelism so total checkouts stay proportional to pool capacity, and drop the unused System.Collections.Generic import.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 62d8a5ff-8bd4-4780-9715-748ea0d950aa
Copilot AI review requested due to automatic review settings July 16, 2026 20:03

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 3 comments.

Comment on lines +249 to +254
int burstCount = 5;
int tasksPerBurst = Parallelism / burstCount;
if (tasksPerBurst < 1)
{
tasksPerBurst = 1;
}

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 8787fceBurstyTrafficPattern now spins up Parallelism concurrent tasks per burst (instead of Parallelism / burstCount), so the parameter drives actual observed concurrency.

Comment on lines +109 to +112
while (reader.Read())
{
_ = reader.GetString(0);
}

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch — switched the JSON sync path to reader.GetSqlJson(0).Value in 8787fce so we exercise the SqlJson accessor rather than the shared string path. VARCHAR still uses GetString(0).

Comment on lines +122 to +125
while (await reader.ReadAsync())
{
_ = await reader.GetFieldValueAsync<string>(0);
}

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same treatment on the async path in 8787fce: JSON uses reader.GetSqlJson(0).Value (JSON handling cost), VARCHAR keeps GetFieldValueAsync<string>(0).

- JsonVsVarcharReadRunner: use SqlDataReader.GetSqlJson(0).Value for the
  JSON case (sync and async) so the benchmark exercises the SqlJson
  accessor path implicated in #3499 instead of the shared GetString /
  GetFieldValueAsync<string> path used for VARCHAR.
- ConnectionPoolStressRunner.BurstyTrafficPattern: spin up Parallelism
  concurrent tasks per burst instead of Parallelism/burstCount, so the
  configured Parallelism parameter actually drives observed concurrency.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 62d8a5ff-8bd4-4780-9715-748ea0d950aa
Copilot AI review requested due to automatic review settings July 16, 2026 20:26

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.

Comment on lines +72 to +82
var behavior = UseSequentialAccess ? CommandBehavior.SequentialAccess : CommandBehavior.Default;

using var conn = new SqlConnection(_connectionString);
conn.Open();
using var cmd = new SqlCommand(_query, conn);
using var reader = cmd.ExecuteReader(behavior);
while (reader.Read())
{
_ = reader.GetString(0);
}
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area\Tests Issues that are targeted to tests or test projects Performance 📈 Issues that are targeted to performance improvements.

Projects

Status: Waiting for customer

Development

Successfully merging this pull request may close these issues.

5 participants