Expand connection pool benchmark coverage for pool implementation A/B testing#4459
Expand connection pool benchmark coverage for pool implementation A/B testing#4459mdaigle wants to merge 6 commits into
Conversation
… testing Adds tooling and benchmarks to evaluate the new ChannelDbConnectionPool (UseConnectionPoolV2) against the legacy WaitHandleDbConnectionPool, aligned with the design goals in issue #3356 (parallel connection opening, async best practices / low threadpool pressure, minimal lock contention). - Add a process-level `UseConnectionPoolV2` config flag (runnerconfig.jsonc -> Config -> Program) that sets the AppContext switch once at startup. The pool implementation switch is read and cached when the first pool is created, so it cannot be toggled per iteration; comparing the two pools requires two runs (flag false, then true). This applies to every pool benchmark in the suite. - Add ConnectionPoolChurnRunner: single-threaded, no-contention open/close on a warm pool. Isolates raw per-checkout CPU and allocation cost (the low-noise counterpart to the parallel/contention runners). - Add ConnectionPoolContentionRunner: steady-state open/query/close with pure sync vs pure async workers and a large (no-wait) vs small (back-pressure) pool axis, focused on the waiter wake path and threadpool pressure. Leaves the existing ConnectionPoolStressRunner and ParallelAsyncConnectionRunner untouched; the new flag lets them all be run against either pool implementation. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Expands the Microsoft.Data.SqlClient.PerformanceTests benchmark suite to better compare the legacy WaitHandleDbConnectionPool vs the new ChannelDbConnectionPool (via Switch.Microsoft.Data.SqlClient.UseConnectionPoolV2) by adding a process-level config flag and two additional pool-focused runners.
Changes:
- Adds
UseConnectionPoolV2torunnerconfig.jsonc+Configand wires it inProgramto set the AppContext switch at process startup. - Introduces
ConnectionPoolChurnRunnerfor single-thread warm-pool checkout/return cost measurement. - Introduces
ConnectionPoolContentionRunnerfor concurrent steady-state open/query/close throughput across sync vs async and pool size contention axes.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Microsoft.Data.SqlClient/tests/PerformanceTests/runnerconfig.jsonc | Adds UseConnectionPoolV2 flag and enables configs for the two new pool runners. |
| src/Microsoft.Data.SqlClient/tests/PerformanceTests/Program.cs | Sets UseConnectionPoolV2 AppContext switch early and registers the new runners. |
| src/Microsoft.Data.SqlClient/tests/PerformanceTests/Config/Config.cs | Adds a documented UseConnectionPoolV2 config field and runner job entries. |
| src/Microsoft.Data.SqlClient/tests/PerformanceTests/BenchmarkRunners/ConnectionPoolContentionRunner.cs | New benchmark runner for steady-state pool contention behavior under sync/async workloads. |
| src/Microsoft.Data.SqlClient/tests/PerformanceTests/BenchmarkRunners/ConnectionPoolChurnRunner.cs | New benchmark runner for low-noise single-thread warm-pool churn cost. |
- Split SteadyStateOpenQueryClose into separate async/sync Task.Run paths (Task.Run(Func<Task>) vs Task.Run(Action)) so the sync branch does not create an async state machine, removing overhead unrelated to pool behavior. - Wrap WarmPool connections in try/finally and dispose each SqlConnection after closing to prevent retaining objects until GC each iteration, which would add allocation noise to the benchmark measurements. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
src/Microsoft.Data.SqlClient/tests/PerformanceTests/BenchmarkRunners/ConnectionPoolContentionRunner.cs:113
- In the async mode, wrapping each worker in Task.Run forces work onto the thread pool and adds scheduler/queueing overhead that can dominate the measurement and inflate ThreadingDiagnoser metrics (e.g., Completed Work Items). Since this runner is meant to highlight the async waiter/wake differences between pool implementations, it should start the async workers without Task.Run (e.g., an immediately-invoked async lambda) so the only thread-pool usage comes from actual async continuations.
if (Async is AsyncBehavior.Async)
{
tasks[i] = Task.Run(async () =>
{
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4459 +/- ##
==========================================
- Coverage 65.83% 63.55% -2.28%
==========================================
Files 287 283 -4
Lines 43763 66821 +23058
==========================================
+ Hits 28812 42470 +13658
- Misses 14951 24351 +9400
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
benrr101
left a comment
There was a problem hiding this comment.
I like it - I think it's pretty straightforward!
Address review feedback: remove the AsyncBehavior param from the churn and contention runners and expose dedicated sync and async benchmarks. The previous single async-signed method forced the sync path to pay async state-machine overhead, distorting per-operation allocation measurements. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Address review feedback: the switch is always set from config (true or false), not only when the flag is enabled. Reword the comment accordingly. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Address review feedback: add a MaxPoolSize (100) greater than Parallelism (50) so the contention runner covers all three regimes — pool larger than, equal to, and smaller than the worker count. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
src/Microsoft.Data.SqlClient/tests/PerformanceTests/BenchmarkRunners/ConnectionPoolContentionRunner.cs:121
- This benchmark also doesn't need an
asyncstate machine at the top level; it can returnTask.WhenAll(tasks)directly. That keeps the worker behavior the same while reducing per-invocation allocations/overhead.
[Benchmark]
public async Task SteadyStateOpenQueryCloseAsync()
{
var tasks = new Task[Parallelism];
for (int i = 0; i < Parallelism; i++)
Address review feedback: the contention benchmark methods only build a task array and await Task.WhenAll, so they don't need an async state machine. Return the Task directly to avoid an extra allocation per invocation. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Summary
Builds on the connection pool benchmarks added in #4447 (now merged to
main) to make the suite useful for evaluating the newChannelDbConnectionPool(UseConnectionPoolV2) against the legacyWaitHandleDbConnectionPool, aligned with the design goals in #3356: parallel connection opening, async best practices (low threadpool pressure), and minimal lock contention.What's added
UseConnectionPoolV2config flag (runnerconfig.jsonc->Config->Program). It sets theSwitch.Microsoft.Data.SqlClient.UseConnectionPoolV2AppContext switch once at startup.false(legacy) thentrue(V2). This flag applies to every pool benchmark in the suite, including the existingConnectionPoolStressRunnerandParallelAsyncConnectionRunner.ConnectionPoolChurnRunner— single-threaded, no-contention open/close on a warm pool. Isolates the raw per-checkout CPU and allocation cost of the pool's acquire/return path (the low-noise counterpart to the parallel/contention runners; the most sensitive measure of per-op allocations).ConnectionPoolContentionRunner— steady-state open ->SELECT 1-> close with pure-sync vs pure-async workers and a large (no-wait) vs small (back-pressure) pool axis. Focuses on the waiter wake path and threadpool pressure (watchCompleted Work Items/Lock Contentions).The existing
ConnectionPoolStressRunnerandParallelAsyncConnectionRunnerare left untouched — the new flag simply lets them all be run against either pool implementation.Not micro-benchmarked
Rate limiting (#4396), pruning (#4304), shutdown drain (#4302), and connection resiliency (#4415) are time/behavior-based and belong in reliability tests (#3667), not hot-path microbenchmarks.
Validation
PerformanceTests(net8.0, Release) — 0 warnings / 0 errors.falseandtrue; V2 showed ~3.5-4.3x faster parallel burst-open and materially lower asyncCompleted Work Items, consistent with Feature | Redesign the SqlClient Connection Pool to Improve Performance and Async Support #3356's goals.Checklist
Related: #3356, #601, #979, #4447