Add perf experiment pipeline and fix v2 connection pool regressions

[mdaigle]

Description

Two related changes: a new perf pipeline for A/B testing AppContext switches, and the first fix for regressions it surfaced in the v2 connection pool.

Perf experiment pipeline

The existing perf pipelines cover two use cases: PR vs main, and main vs a published baseline. In both, any AppContext switches apply to baseline and current alike, so they cannot measure the effect of the switch itself.

This adds a third use case: run the same commit against itself with a perf-sensitive switch on in one variant and off in the other.

• New eng/pipelines/perf/sqlclient-perf-experiment.yml with a switchUnderTest dropdown (default UseConnectionPoolV2).
• run-perf-tests.sh / .ps1 take a general --switch-under-test / -SwitchUnderTest.
• interleave_perf.py applies a per-variant runner-config override.

Two things worth calling out:

• This pipeline never ingests into Kusto. Its two variants are the same commit, so the results are not comparable to the trend data the other pipelines produce and would pollute it.
• Two processes are required. InProcessEmitToolchain in BenchmarkConfig.cs pins benchmarks to the host process, so AppContext switches cannot be varied within a single BenchmarkDotNet run.

The pipeline drops the useManagedSni / useConnectionPoolV2 / useOptimizedAsyncBehaviour parameters the other pipelines expose. ADO boolean parameters always emit a value, so leaving them in would fire the "flag ignored" warning on every run. The runnerconfig.jsonc defaults already match what those parameters defaulted to.

v2 connection pool fast path

Running the above with switchUnderTest: UseConnectionPoolV2 showed large regressions on open/close-heavy benchmarks: OpenAsyncConnection +273%, RapidOpenCloseSingleThreadAsync +98%, RapidFireOpenClose +46-85% with allocation deltas of +110-212%.

Three causes, all on the path taken when the pool already holds a usable connection:

• Every async open dispatched to the thread pool via Task.Run with no inline attempt first. v1 tries a non-blocking acquisition on the caller's thread (allowCreate: false) and only queues a pending request on a miss. The 3.6 us to 13.3 us jump on OpenAsyncConnection is thread pool round-trip latency for trivial work.
• GetInternalConnection creates a timer-backed CancellationTokenSource before it knows whether it will ever wait. That is an allocation plus a TimerQueueTimer registration, which also contends a shared lock at higher parallelism. This is the allocation delta on the uncontended benchmarks above; the pool-stress runners are a separate cause, covered below.
• GetInternalConnection is an async method, so it allocates a Task<DbConnectionInternal> even when it completes synchronously.

TryGetPooledConnectionInline performs the transacted-store and idle-channel lookups that GetInternalConnection begins with and returns null the moment neither can satisfy the request. Both the sync and async entry points try it first, so the common case avoids all three costs at once. It deliberately never calls OpenNewInternalConnection, so no caller thread blocks on network I/O.

Remaining differences after the fix

Latest run: 173 benchmarks, 10% threshold, 3 interleaved confirmation runs. 10 confirmed regressions, 3 unconfirmed, 51 improvements. 38 benchmarks flag on time or allocation. Allocation is reported only and never gates the build (compare_perf.py sets status from meanDeltaPct alone), so of the 31 allocation increases above 10%, 24 are on benchmarks that got faster.

Scoping first. Benchmarks with no pool checkout in the measured body (37) have a max allocation delta of +0.05%, and those amortising one checkout over real work (67) max at +1.41%. Zero above 10% in either group; all 31 are checkout-dominated. ParallelAsyncConnectionRunner parameterises pooling directly, so it isolates the pool from everything else in the same benchmark:

concurrency	alloc Δ, Pooling=False	alloc Δ, Pooling=True
10	+0.04%	+113.16%
50	+0.11%	+120.56%
100	+0.01%	+70.55%

The 38 flagged benchmarks break down as 30 concurrent pool growth, 3 threadpool saturation, 1 saturated async waiter allocation, and 4 noise. Each is covered below.

A. Concurrent pool growth (30 of 38) — benchmark defect, now fixed. The flagged ConnectionPoolStressRunner cases plus ParallelAsyncConnectionRunner.OpenConnectionsConcurrently. Both runners ClearAllPools() in [IterationCleanup], so every iteration re-established physical connections inside the measured body. v1 serialises pool growth behind a Semaphore(1, 1); v2 deliberately does not, so it creates more physical connections, finishes sooner, and allocates more. A physical connection costs the same in both pools (v1 57,275 B, v2 57,701 B), and dividing each benchmark's allocation delta by its Login7Count delta lands on 55.5-59 KB in every shape tested.

Rather than explain that away, ConnectionPoolStressRunner has been fixed (ab21302): it now pre-warms the pool to full capacity in [GlobalSetup] and no longer clears it between iterations, so its bodies only exercise checkout and return. Establishing a connection costs milliseconds against microseconds for a pooled checkout, so any creation left in the measured body swamps the cost the class exists to measure. The 27 ConnectionPoolStressRunner rows quoted here therefore predate that fix and will be re-measured on the next pipeline run. ParallelAsyncConnectionRunner is deliberately left as-is: it reproduces #601/#979, where concurrent open storms against a cold pool are the actual subject, and it has Pooling=false variants.

Three smaller benchmark defects were fixed alongside it, all of which also invalidate the stress rows quoted above:

• RapidFireOpenCloseSync added and RapidFireOpenClose renamed to RapidFireOpenCloseAsync (2ac2b2e, 2959100). The concurrent runner measured this workload on the async path only, so a sync/async split there could not be told apart from a scheduling difference. Result rows quoted elsewhere in this description predate the rename and appear under the old name. The old name still exists on main, so a PR-vs-main run will report this row without a delta rather than comparing it — which is correct, since the benchmark now pre-warms the pool where main clears it, and a delta across that change would be spurious.
• ConnectionPoolChurnRunner parameterised on pool depth (2104d10). It was pinned at one pooled connection, which is the one depth at which the two pools cannot differ in reuse order: v1 pops idle connections from a ConcurrentStack (LIFO) and reuses the connection just released, while v2 reads from an unbounded Channel (FIFO) and cycles through all of them. The runner now covers depth 1 and 100.
• PoolExhaustionRecovery now sizes itself as MaxPoolSize + Parallelism (9ede8e0). It was Math.Max(Parallelism, MaxPoolSize * 2), and since that second term is 100 or 200 while Parallelism never exceeds 25, the parameter had no effect: six configurations were only two distinct workloads. Parallelism is now the oversubscription amount, which is the depth of the queue of waiting callers.

ConnectionPoolRampRunner was added to test that claim rather than argue it. It keeps the cold pool but makes every caller hold its connection until all have connected, so both pools must create the same number of connections and the only remaining variable is how fast they get there:

	time Δ	alloc Δ
ColdStartRamp P=10 / 25 / 50	−75.23% / −80.62% / −82.20%	+0.08% / +0.58% / +0.64%
ColdStartRampAsync P=10 / 25 / 50	−73.42% / −80.93% / −82.89%	−2.62% / −0.17% / +0.80%

Holding connection count constant makes the allocation delta vanish and leaves v2 4-6x faster. That was the same behaviour RapidFireOpenClose scored as a 33-75% regression, because it held connections for zero time and so measured a burst that never needed the extra connections.

Two independent checks already agreed that the checkout path itself got faster, before any benchmark change. ConnectionPoolChurnRunner runs RapidFireOpenClose's exact inner loop — new SqlConnection / OpenAsync / dispose — single-threaded against a warm pool that is never cleared, and v2 wins there: −31.27% async and −31.54% sync, with allocation at +3.51% / +4.92% rather than +47–139%. (Those were measured at pool depth 1, before the depth axis above was added.) SqlConnectionRunner's pooled opens agree at −29% to −47%.

One caveat on reading the old stress numbers precisely: every Parallelism value (10, 20, 25) is below every MaxPoolSize value (50, 100), and the Math.Max(20, ...) floor leaves Parallelism 20 and 25 running identical workloads at both MaxPoolSize values. Those duplicate pairs report deltas 22.5 pp and 19.0 pp apart (+75.18% vs +52.68%, and +59.16% vs +40.12%), which puts this benchmark's noise floor at twice the 10% threshold.

B. Threadpool saturation on the sync wait path (3 of 38). SteadyStateOpenQueryClose P=50/Max=10 at +136.27%. A sync waiter blocks in mres.Wait(), and with AllowSynchronousContinuations off the returning thread must queue the wake into a pool whose workers are all blocked, so it waits on thread injection. Two controls isolate that:

variant, P=50 / Max=10	time Δ	alloc Δ
SteadyStateOpenQueryClose (threadpool, sync)	+136.27%	+5.87%
SteadyStateOpenQueryCloseDedicatedThreads (sync)	+5.61%	+8.00%
SteadyStateOpenQueryCloseAsync (threadpool)	−4.20%	+10.27%

Allocation is comparable across all three while time differs by 140 points, so the regression is not the pool's connection handling. ConnectionPoolThreadPoolPressureRunner varies only the thread floor and shows the same thing: MinWorkerThreads=8 gives +57.48% (3/3), MinWorkerThreads=128 gives +11.23% (2/3). The ample-pool variants stay under the threshold (Max=50 is −1.80%, Max=100 is +7.91%), so this is specific to Parallelism ≫ MaxPoolSize.

Accepted as an application-configuration boundary: an application should keep parallelism below the threadpool worker count, and pre-warming the threadpool is not the driver's job. Enabling AllowSynchronousContinuations was measured (−7.8%) and rejected, because OpenNewInternalConnection sits in the same retry loop and an inline resume could run a TCP connect, TLS handshake and login on a caller's Close() thread.

C. Saturated async waiter allocation (1 of 38). SteadyStateOpenQueryCloseAsync P=50/Max=10, +10.27% allocation while 4% faster. The only genuine per-operation allocation regression, and the only Async row with a meaningful delta (Max=50 is +0.19%, Max=100 is +0.77%). A saturation sweep holds flat at +24 B/op down to maxPool=25, then jumps to +1,241 (maxPool=10) and +1,481 (maxPool=5) on the async path only; an isolated microbenchmark predicted +10.05% for this shape against the +10.27% measured. 392 B of it is passing a cancellable token to ReadAsync: 104 B defeats UnboundedChannel's cached reader, 192 B is the CTS, 96 B its timer. Sync escapes because _syncOverAsyncSemaphore caps concurrent readers.

Not fixed here. Deferring CTS creation to the first blocking wait was implemented and measured: it saves 24 B of 1,481, because a saturated caller always blocks and so always needs the token. Removing the 392 B means reading with CancellationToken.None, which needs an explicit waiter queue so a timed-out waiter can deregister without stranding the connection a pending read would swallow. That is the same restructuring B needs, so it is one workstream rather than two.

D. Noise (4 of 38). The async large-data read benchmarks contribute the +68.23% (2/3) and two 1/3 flags, and SequentialXmlReadRunner.ReadXml adds a fourth at +11.55% (1/3). Across all 16 large-data rows the deltas scatter from −13.92% to +68.23% with allocation pinned at ≈0.00% throughout, and three of those rows are flagged improvements of comparable size; the same 5 MB payload reads +68.23% with a 1 MB buffer and −13.92% with an 8 KB buffer. Both runners take one checkout and then read, so the switch has nothing to act on after connect. The previous run's two 1/3 flags (MarsOverheadRunner, BeginTransactionRunner) did not reproduce. (These ran as AsyncLargeDataReadRunner; that runner has since been reorganised on main into LargeDataReadRunner/{Plaintext,AlwaysEncrypted}.)

Two caveats worth stating. SteadyStateOpenQueryClose P=50/Max=10 moved from +66.61% to +136.27% between runs (both 3/3): this is a tail-latency effect, not a shifted median, so the magnitude is not a stable number to quote. And the MinWorkerThreads=128 control lands at +11.23% rather than parity, because its 50 blocked waiters are still threadpool threads competing for scheduling, which the dedicated-thread variant avoids.

Separately, I investigated and ruled out redundant liveness probing as a cause of the original regressions. v2 calls IsLiveConnection up to 3x per cycle vs v1's 1x, but IsConnectionAlive is gated by a 5 ms window and a successful check resets the timer, so the extra calls collapse to a few DateTime.UtcNow reads.

Issues

N/A

Testing

Two unit tests added to ChannelDbConnectionPoolTest:

• GetConnectionAsync_WithIdleConnection_ShouldCompleteInline is the meaningful one. It asserts the TaskCompletionSource is already completed when TryGetConnection returns, which is impossible to observe if the work was dispatched via Task.Run. Verified it fails without the fix.
• GetConnection_WithIdleConnection_ShouldReturnInline covers the sync path.

The full connection pool unit test suite passes: 359 tests, green on 3 consecutive runs after merging main.

Two fixes came out of that merge and the review pass:

• Soft connects were not counted on the new fast path. TryGetPooledConnectionInline activated the connection without recording the request, so a fast-path checkout went uncounted while its matching return still emitted a soft disconnect. Main's new DbConnectionPoolInstrumentationTest caught it (152 counted instead of 400). The count now happens immediately before activation, matching GetInternalConnection.
• ConnectionPoolRampRunner could hang. Both ramp benchmarks only reached their rendezvous on the success path, so a caller that threw before signalling left the rest waiting forever, which would stall the perf job. Signalling now happens in a finally and both waits are bounded.

Not automated: the perf improvement itself. Plan is to re-run the experiment pipeline on this branch with switchUnderTest: UseConnectionPoolV2 and confirm the async cases move back toward parity.

One gap worth flagging: the full unit test suite hangs in SimulatedServerTests on macOS. I confirmed this is pre-existing by reproducing the identical hang on a clean tree, and those tests do not use the v2 pool.

Guidelines

Please review the contribution guidelines before submitting a pull request:

• Contributing
• Code of Conduct
• Best Practices
• Coding Style
• Review Process

[mdaigle]

https://dev.azure.com/SqlClientDrivers/ADO.Net/_build/results?buildId=167208&view=results

[Copilot]

Pull request overview

Adds a new perf “switch experiment” pipeline to A/B test a single runner-config/AppContext switch on the same commit, and applies an optimization to the v2 channel-based connection pool to remove avoidable allocations and thread-pool dispatch on the idle-connection fast path.

Changes:

• Introduces sqlclient-perf-experiment.yml to run baseline/current as the same source with exactly one switch flipped (no Kusto ingestion by design).
• Updates perf runner scripts (run-perf-tests.sh / .ps1) and the interleaving orchestrator to support per-variant RUNNER_CONFIG overrides for switch A/B.
• Adds TryGetPooledConnectionInline to ChannelDbConnectionPool.TryGetConnection to satisfy idle/transacted requests inline and avoid Task.Run/CTS/Task<T> allocations; adds unit tests covering sync and async inline completion.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs	Adds unit tests verifying idle-connection requests complete inline for both sync and async paths.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs	Adds a pooled-connection inline fast path (TryGetPooledConnectionInline) to avoid async state machine + CTS + thread-pool dispatch when an idle/transacted connection is immediately available.
eng/pipelines/perf/sqlclient-perf-experiment.yml	New manual perf pipeline that runs the same commit twice with one switch forced off vs on.
eng/pipelines/perf/scripts/run-perf-tests.sh	Adds --switch-under-test mode, generates per-variant runner configs, and wires them into interleaved/sequential runs.
eng/pipelines/perf/scripts/run-perf-tests.ps1	Windows equivalent of switch-under-test A/B mode, including per-variant runner config generation and plumbing.
eng/pipelines/perf/scripts/interleave_perf.py	Adds optional per-variant environment overrides so baseline/current subprocesses can use different RUNNER_CONFIG values.
eng/pipelines/perf/README.md	Documents the experiment pipeline, its constraints, and why it must not be ingested into Kusto.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs:1561

• GetConnection_ConcurrentHeldLoad_GrowsPoolToDemand asserts factory.Created == parallelism, but RunWorkersAsync does not synchronize worker start. Because tasks can begin at different times, some workers may start after others have already returned a connection and end up reusing it, causing factory.Created to be < parallelism and making this test timing/scheduling-sensitive (potentially flaky in CI). Consider adding a start barrier (e.g., CountdownEvent + ManualResetEventSlim) so all workers contend concurrently before any can return, or relax the assertion to a range that still detects the “serialized to 1 connection” failure mode without requiring perfect simultaneity.

            // Act: workers hold their connections, so none can be reused and the pool must grow.
            await RunWorkersAsync(pool, parallelism, iterationsPerWorker, holdMilliseconds: 25);

            // Assert: one connection per concurrent caller, and no more.
            Assert.Equal(parallelism, factory.Created);

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

Suppressed comments (1)

eng/pipelines/perf/scripts/run-perf-tests.ps1:150

• The comment above the baseline-selector validation still says there are "two baseline selectors" and that "the two baseline selectors are mutually exclusive", but this script now supports three selectors (BaselineVersion, BaselineSourceRef, and SwitchUnderTest). This makes the comment misleading for future maintenance and review.

# The two baseline selectors describe different builds of the same "baseline" pass, so requesting
# both is always a mistake; fail fast rather than silently honouring one of them.
if ((-not [string]::IsNullOrEmpty($BaselineVersion)) -and (-not [string]::IsNullOrEmpty($BaselineSourceRef))) {
    throw "-BaselineVersion and -BaselineSourceRef are mutually exclusive."

[Copilot]

Pull request overview

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

Suppressed comments (1)

eng/pipelines/perf/README.md:270

• The README states that the switch-experiment baseline requires “no second build at all”, but the scripts only reuse a single build in the interleaved path. In sequential mode, run-perf-tests.sh/.ps1 still run the baseline and current passes separately (and therefore build twice). Clarifying this avoids misleading users who select sequential mode.

- **Baseline (switch experiment)**: no second build at all — `--switch-under-test` measures one
  source tree twice, so the scripts build the `current` variant once and point both passes at it,
  differing only in the runner config each pass is handed. Used by the experiment pipeline.

[Copilot]

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated no new comments.

Suppressed comments (2)

Previously missed (2) — in code that hasn't changed since the last review.

eng/pipelines/perf/README.md:279

• The README claims the switch-experiment baseline does “no second build at all”, but the perf scripts only reuse a single build output in interleaved mode. In sequential mode (RunMode=sequential), both run-perf-tests.sh and run-perf-tests.ps1 still build the baseline and current passes separately (see run-perf-tests.sh run_pass builds the project per pass, and run-perf-tests.ps1 Invoke-PerfPass likewise). This doc line should be qualified so users don’t assume sequential runs avoid the second build.

- **Baseline (switch experiment)**: no second build at all — `--switch-under-test` measures one
  source tree twice, so the scripts build the `current` variant once and point both passes at it,
  differing only in the runner config each pass is handed. Used by the experiment pipeline.

eng/pipelines/perf/sqlclient-perf-experiment.yml:83

• This comment says “both passes here share ONE build” as a general statement, but that’s only true when benchmarkRunMode is interleaved. In sequential mode, the run scripts still perform a build per pass (baseline then current), even for switch experiments. Tweaking the comment to call out the interleaved-only behavior will keep the pipeline self-documentation accurate.

  # Maximum time (minutes) the template waits for the benchmark run on the VM before timing out.
  # Matches the nightly pipeline rather than the PR pipeline: both passes here share ONE build (the
  # source is identical on both sides), so there is no second driver build to pay for.

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

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

Suppressed comments (2)

Previously missed (2) — in code that hasn't changed since the last review.

eng/pipelines/perf/scripts/run-perf-tests.sh:832

• When -SwitchUnderTest is used with the legacy sequential run mode, this path still runs run_pass twice (baseline + current). run_pass always performs a dotnet build, so the same source is rebuilt twice even though switch A/B is intended to compare one build against itself. This adds several minutes of avoidable VM time when benchmarkRunMode=sequential and also conflicts with the experiment pipeline’s “single build” rationale.

Consider either (a) forbidding sequential mode when --switch-under-test is set, or (b) reworking the sequential path to build once (like the interleaved switch A/B path already does) and run two passes from the same output directory while only varying RUNNER_CONFIG.

    run_pass "baseline" "${baselineProject}" ${baselineBuildArgs[@]+"${baselineBuildArgs[@]}"}
    if [[ -n "${switchUnderTest}" ]]; then
        export RUNNER_CONFIG="${CURRENT_RUNNER_CONFIG}"
    fi
    run_pass "current" "${PERF_PROJECT}"

eng/pipelines/perf/scripts/run-perf-tests.ps1:811

• In legacy sequential mode, -SwitchUnderTest still invokes Invoke-PerfPass twice (baseline + current). Invoke-PerfPass always performs a dotnet build, so the same source is rebuilt twice even though the switch experiment is meant to compare a single build against itself with only RUNNER_CONFIG flipped.

Consider either disallowing sequential mode when -SwitchUnderTest is set, or refactoring the sequential path to reuse a single Build-Variant output directory (similar to the interleaved switch A/B optimization) and only run two processes with different RUNNER_CONFIG values.

    if (-not [string]::IsNullOrEmpty($SwitchUnderTest)) { $env:RUNNER_CONFIG = $BaselineRunnerConfig }
    Invoke-PerfPass "baseline" $BaselineProject $BaselineBuildArgs
    if (-not [string]::IsNullOrEmpty($SwitchUnderTest)) { $env:RUNNER_CONFIG = $CurrentRunnerConfig }
    Invoke-PerfPass "current" $PerfProject @()

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/Microsoft.Data.SqlClient/tests/PerformanceTests/BenchmarkRunners/ConnectionPoolContentionRunner.cs:117

• This introduces an unused local result, which will raise CS0219 (assigned but never used). The repo treats warnings as errors, so this can break the build. If the intent is just to execute the query, discard the result instead of storing it.

                        using var conn = new SqlConnection(_connectionString);
                        conn.Open();
                        using var cmd = conn.CreateCommand();
                        cmd.CommandText = "SELECT 1";
                        var result = cmd.ExecuteScalar();
                        // Dispose returns the connection to the pool.