diff --git a/.github/instructions/connection-pooling.instructions.md b/.github/instructions/connection-pooling.instructions.md index 6464fdedd0..e845e2e079 100644 --- a/.github/instructions/connection-pooling.instructions.md +++ b/.github/instructions/connection-pooling.instructions.md @@ -124,13 +124,20 @@ private readonly ChannelWriter _idleConnectionWriter; await _idleConnectionReader.WaitToReadAsync(token); ``` -### Sync Over Async Protection +### Sync Over Async Blocking ```csharp -// Prevent thread pool starvation -private static SemaphoreSlim _syncOverAsyncSemaphore = - new(Math.Max(1, Environment.ProcessorCount / 2)); +// Block via Task, not an opaque primitive, so the thread pool learns this +// thread is stalled and can inject a worker to run the read's continuation. +return _idleChannel.ReadAsync(cancellationToken).AsTask().GetAwaiter().GetResult(); ``` +The idle channel is created without `AllowSynchronousContinuations`, so the continuation +that completes a pending read is queued to the thread pool. Blocking on an opaque primitive +(`ManualResetEventSlim`, `SemaphoreSlim`) hides the stall from the thread pool, so under +saturation every parked thread waits on a continuation the pool does not know it needs to +run. Do not reintroduce a semaphore around this wait: it cannot stop sync work from +consuming thread pool threads, since waiting on it blocks just as opaquely. + ## Best Practices ### Application Design diff --git a/eng/pipelines/perf/README.md b/eng/pipelines/perf/README.md index be925f5431..09ddfe1831 100644 --- a/eng/pipelines/perf/README.md +++ b/eng/pipelines/perf/README.md @@ -12,7 +12,8 @@ database. | ---- | ------- | | `sqlclient-perf-pipeline.yml` | The main (manual/nightly) pipeline. Extends `v1/Perf.Test.Job.yml@PerfTemplates`. Baseline = released NuGet package; ingests into Kusto. | | `sqlclient-perf-pr-pipeline.yml` | PR pipeline. Same template, same scripts, same options; baseline = **`main` branch source**; **no Kusto ingestion**. | -| `scripts/run-perf-tests.sh` | Linux on-VM entry point: install SDK, create DB, run benchmarks (interleaved or sequential), compare. Baseline is either a released package (`--baseline-version`) or another git ref's source (`--baseline-source-ref`). | +| `sqlclient-perf-experiment.yml` | Experiment pipeline. Same template, same scripts; both passes build the **same source** and differ only in one runner-config switch; **no Kusto ingestion**. | +| `scripts/run-perf-tests.sh` | Linux on-VM entry point: install SDK, create DB, run benchmarks (interleaved or sequential), compare. Baseline is a released package (`--baseline-version`), another git ref's source (`--baseline-source-ref`), or the same source with one runner-config switch flipped off (`--switch-under-test`). | | `scripts/run-perf-tests.ps1` | Windows equivalent (ProcessorAffinity instead of `taskset`). | | `scripts/interleave_perf.py` | Interleaved + best-of-N orchestrator: runs each unit baseline↔candidate back-to-back and confirms regressions across N passes. | | `scripts/compare_perf.py` | Compares baseline vs current BenchmarkDotNet JSON → delta (md + json). Reused by the orchestrator. | @@ -151,6 +152,117 @@ the comparison (and one removed by the PR as `removed`) instead of failing the r share the single generated runner config (`RUNNER_CONFIG` / `DATATYPES_CONFIG` env vars), so connection string and behaviour flags are identical on both sides. +## Experiment pipeline (`sqlclient-perf-experiment.yml`) + +The three perf pipelines are the same benchmarks, template and scripts pointed at three different +questions. Two of them vary the **source** under measurement; the third varies the **config**: + +| Question | Pipeline | Baseline | Current | +| --- | --- | --- | --- | +| Has this branch regressed against a released package? | `sqlclient-perf-pipeline.yml` | released NuGet package | queued branch | +| Does my PR regress the branch it merges into? | `sqlclient-perf-pr-pipeline.yml` | `main` source | queued branch | +| What does this switch cost or buy? | `sqlclient-perf-experiment.yml` | queued branch, switch **off** | queued branch, switch **on** | + +`sqlclient-perf-experiment.yml` picks one runner-config switch via the `switchUnderTest` +queue-time parameter (`UseConnectionPoolV2`, `UseOptimizedAsyncBehaviour` or +`UseManagedSniOnWindows`) and runs the baseline pass with it `false` and the current pass with it +`true`. Both passes measure the **same commit** — the branch the run is queued on — so queue it on a +PR branch to ask "what does this switch do to my change?", or on `main` to ask "what does it do to +`main`?". + +Two passes are required because these are `AppContext` switches latched process-wide (for example +`UseConnectionPoolV2` is read and cached the first time a connection pool is created), so they cannot +be toggled between benchmarks within a single process. The pipeline wires that into the existing +comparison machinery — interleaved best-of-N or sequential, via `benchmarkRunMode` — instead of two +ad hoc manual runs. + +Switch-pipeline parameters that differ from the tables above: + +| Parameter | Default | Description | +| --------- | ------- | ----------- | +| `switchUnderTest` | `UseConnectionPoolV2` | The single switch to A/B. Baseline forces it `false`, current forces it `true`. | +| `failIfSwitchSlower` | `false` | Maps onto the scripts' `--fail-on-regression` gate, but means something different here: "switch on is slower" is usually the *result* you queued the run to measure, not a defect. Enable it only when asserting the switch must not be a slowdown (e.g. before flipping its default). | +| `testTimeoutMinutes` | `180` | Same as the main pipeline, not the PR pipeline's `210`: both sides are the same source, so only **one** driver build is needed. | + +The pipeline deliberately does **not** expose `baselineVersion` / `baselineSourceRef` (the run +scripts reject combining those with `--switch-under-test`, since a simultaneous source change would +make the delta unattributable), nor the `useManagedSniOnWindows` / `useOptimizedAsyncBehaviour` / +`useConnectionPoolV2` flags. Every switch except the one under test stays at its checked-in +`runnerconfig.jsonc` value, so the measured difference is attributable to exactly one variable. + +### Why these runs are never ingested into Kusto + +This mode has its own pipeline file, rather than being a flag on the other two, specifically so that +"never ingested" is structural rather than a conditional someone can flip. Ingesting a switch +experiment would corrupt the perf database three ways: + +* **`DerivedRunId` collision.** The ID is `driver|commit|pipelineRunId`. The other two pipelines keep + their two rows distinct because the baseline row carries a *different* commit (`v7.0.2`, or the + baseline ref's sha). Here both passes are the same commit in the same pipeline run, so both rows + would derive the same ID. +* **`PerfRun.Config` is stamped once per run.** `translate_results_to_kusto.sh` builds one + `--config-override` set from the queue-time `CFG_*` values and reuses it for both the baseline and + current rows. That is correct when the config genuinely is shared, but it means the two rows could + not record the differing switch values that are the entire point of the experiment. +* **Trend pollution.** No field marks a row as an experiment — `RunType` is already + `Sequential`/`Interweaved` — so the switch-on pass would be indistinguishable from an ordinary + measurement of the branch and would distort the very trends the other two pipelines exist to + protect. + +The comparison report and the raw BenchmarkDotNet artifacts are published as usual, and the build is +tagged `Switch ` so experiments are identifiable in the ADO build list. + +### Designing benchmarks for switch experiments + +A switch experiment flips behaviour on purpose, so a benchmark that measures that behaviour will +report a regression even when the change is working. `UseConnectionPoolV2` is the motivating example: +`ChannelDbConnectionPool` opens physical connections concurrently, where `WaitHandleDbConnectionPool` +serialises growth behind a `Semaphore(1, 1)`. `ConnectionPoolStressRunner` used to call +`ClearAllPools()` in `[IterationCleanup]`, so every iteration was a cold-start burst in which the +extra parallel opens had nothing to amortise against, and the runner reported the trade-off as a loss +because that was the only thing it could measure. + +That is a benchmark defect rather than something to explain away, so the runner now pre-warms the +pool to full capacity in `[GlobalSetup]` and no longer clears it between iterations. Establishing a +connection costs milliseconds while a pooled checkout costs microseconds, so any creation left in the +measured body swamps the pool cost the runner exists to measure. Keep that separation in mind when +adding a pool benchmark: warm the pool first unless connection establishment is precisely the thing +under test. + +The pipeline has no way to mark a result as acceptable, and deliberately so: a mute is only as good +as the reasoning behind it, and that reasoning belongs in the pull request where a reviewer can +challenge it. Prefer instead to fix the benchmark, or add one that measures the intended behaviour +directly. `ConnectionPoolRampRunner` was added for exactly this reason: it keeps the cold pool but +makes every caller *hold* its connection until all of them have connected, so the pool genuinely needs +N physical connections and the only variable left is how fast it can open them. That rewards +concurrent creation instead of penalising it, and it isolates cold start now that the stress runner no +longer conflates it with checkout cost. + +The same principle applies to how a benchmark schedules its workers. A sync `Open()` that has to +wait blocks whichever thread it runs on, so a pool whose waiter wake-up needs a queued continuation +stalls when every threadpool thread is already blocked; the wake-up waits on thread injection, which +costs about a second each time. Threadpool threads are the realistic case, because sync database +calls in ASP.NET run on them, and they are the only configuration in which that stall is visible. +Benchmarks therefore keep threadpool threads as the default and add dedicated-thread variants +alongside rather than instead: + +- `ConnectionPoolContentionRunner.SteadyStateOpenQueryCloseDedicatedThreads` runs the existing + workload on dedicated threads. A regression in both variants points at the pool; a regression in + only the threadpool variant points at the waiter wake path. +- `ConnectionPoolThreadPoolPressureRunner` pins the threadpool floor via `[Params]`, below the worker + count (starved) and above it (control), so the effect is reproducible instead of depending on + hill-climbing timing. + +This failure mode is tail latency, not a shifted median, so compare distributions. Aggregating with +a per-configuration minimum hides it completely. + +Note what those benchmarks are for. Saturating the thread pool with blocked synchronous calls is an +application configuration problem, not a pool defect: an application should keep its parallelism +below the thread pool's worker count so newly queued work still runs promptly, and pre-warming the +thread pool is the application's responsibility rather than the driver's. These benchmarks exist to +characterise where that boundary sits and to catch it moving, so a delta here is a prompt to check +the boundary has not shifted rather than a bug to fix. + ## Two-pass build model The `PerformanceTests` project references Microsoft.Data.SqlClient two ways, selected by MSBuild: @@ -162,6 +274,9 @@ The `PerformanceTests` project references Microsoft.Data.SqlClient two ways, sel - **Baseline (source)**: no reference switching at all — the baseline ref's own copy of the perf project is built from `../sqlclient-perf-baseline-src`, keeping its default `ProjectReference` to that ref's driver source. Used by the PR pipeline. +- **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. The VM's `NuGet.config` exposes only the governed feed, and CPM rejects multiple unmapped sources (`NU1507`). The baseline pass therefore restores through a **dedicated single-source config** @@ -210,12 +325,39 @@ supplies the isolated dedicated host, the tuned SQL instance, and the disjoint c | Fail loud | Preflight `SELECT 1` before any pass, **and** a post-pass guard that fails the run if a pass produced **zero** benchmark results — so an empty comparison can never be reported green. | | Warm-up | Touches the target DB in the preflight to warm the buffer pool / plan cache before the first measured benchmark. | | Allocator tuning (Linux) | Exports `MALLOC_MMAP_THRESHOLD_=128MiB` and `MALLOC_TRIM_THRESHOLD_=-1` so large-buffer benches (`LargeDataRead`, `SqlBulkCopy`) stop re-`mmap`ing per iteration. | -| Network tuning (Linux) | Best-effort `sysctl` to widen the ephemeral port range and enable `tcp_tw_reuse` for churn benches (`ConnectionPoolStress`, `ParallelAsyncConnection`). Never fails the run. | +| Network tuning | Best-effort widening of the ephemeral port range so connection-churn benches don't stall on `TIME_WAIT`. On Linux: `sysctl` for `ip_local_port_range` plus `tcp_tw_reuse`. On Windows: `netsh int ipv4 set dynamicport tcp` (10000-65535) plus `TcpTimedWaitDelay` (reboot-scoped, so it benefits subsequent runs). Never fails the run. See [Ephemeral ports](#ephemeral-ports-and-connection-churn). | | Diagnostics | Writes `results/diagnostics/`: SQL instance config (MAXDOP, memory, affinity, tempdb files, `@@VERSION`), host CPU topology, and per-pass CPU-clock/thermal telemetry (before/after each pass). | | Regression gate | `failOnRegression` threads `--fail-on-regression`; only a **candidate-slower** delta past the threshold fails, and in interleaved mode only after best-of-N confirmation. Default off. | | Interleaving | In `interleaved` mode the harness runs **one benchmark unit at a time, baseline then candidate back-to-back**, so both sides see the same host state (see below). | | Best-of-N confirmation | A unit flagged in the first interleaved pass is re-run `confirmationRuns` times; a regression is **confirmed** only on a strict majority. Unconfirmed flags are reported but never fail the gate. | +### Ephemeral ports and connection churn + +The suite opens far more *physical* TCP connections than the benchmark names suggest, so the client's +ephemeral port supply is a real constraint rather than a theoretical one: + +| Source | Physical connects | Why | +| ------ | ----------------- | --- | +| `SqlConnectionRunner` (`Pooling=False` cases) | ~6,600 | `Pooling` is a `[Params(true, false)]` axis, so half the cases bypass the pool entirely and every operation is a fresh connect. 4 cases x (5 warmup + 50 iterations) x 30 invocations, back-to-back with no delay. | +| `ConnectionPoolRampRunner` | ~2,700 | `[IterationSetup]` calls `ClearAllPools()`, so every iteration ramps from a cold pool. | +| `ConnectionPoolStressRunner` | ~3,100 | `MinPoolSize = MaxPoolSize`, so each of the 42 cases fills a fresh pool. | + +Every closed socket then holds its port in `TIME_WAIT`. Untuned Windows allows 16,384 dynamic ports +(49152-65535) with a 120 s `TcpTimedWaitDelay` — about **136 sustained connects/sec**, which the +suite exceeds by orders of magnitude. Once the range drains, each subsequent connect blocks until a +port is released. + +This matters more here than in a typical harness because `runnerconfig.jsonc` pins `InvocationCount` +for every runner. That disables BenchmarkDotNet's pilot stage, so nothing renormalises wall-clock +time: a port stall lands directly in the reported per-operation numbers rather than being absorbed by +a smaller invocation count. + +Both platforms are tuned for this, and results are **not comparable across platforms without it** — +an untuned Windows run measures its own port pressure as much as it measures the driver. If a Windows +run looks uniformly slow, check `results/diagnostics/tcp-dynamicport.txt` to confirm the widening +applied (it needs an elevated agent), and sample `netstat -an | find /c "TIME_WAIT"` during the run: +a count parked near the top of the range means the client is port-starved. + ### Interleaving + best-of-N (run model) `benchmarkRunMode` selects how the two variants are measured: @@ -323,6 +465,18 @@ translated NDJSON as the `perf-kusto-payloads` artifact for manual/backfill inge 3. After the run, review the **run summary** (comparison, labelled `@`) and the `perf-results` artifact. The build is tagged **`Baseline `**. +### Running the experiment pipeline + +1. Open the **experiment** performance test pipeline (`sqlclient-perf-experiment.yml`) in Azure + DevOps and select **Run pipeline**. +2. Choose the branch whose behaviour you want to measure — `main` to characterise the switch on its + own, or a PR branch to characterise it against that change — and pick `switchUnderTest`. There is + no baseline selector: the baseline *is* this branch with the switch off. No Kusto configuration is + involved; these results are never ingested (see [Why these runs are never ingested into + Kusto](#why-these-runs-are-never-ingested-into-kusto)). +3. After the run, review the **run summary** (comparison, labelled `=false`) and the + `perf-results` artifact. The build is tagged **`Switch `**. + ## Troubleshooting | Symptom | Likely cause / fix | @@ -331,6 +485,8 @@ translated NDJSON as the `perf-kusto-payloads` artifact for manual/backfill inge | Baseline restore fails to find MDS | `baselineVersion` isn't a published NuGet.org version, or the VM has no outbound access to `api.nuget.org`. | | Baseline pass fails to compile: `CS1061 ... does not contain a definition for ` | A benchmark calls an MDS API newer than `baselineVersion`. Guard it with the `MDS_GE_` constants and add an older fallback — see [Benchmarks must compile against the oldest baseline](#benchmarks-must-compile-against-the-oldest-baseline). | | No comparison / summary | The baseline pass was skipped (empty `baselineVersion` / `baselineSourceRef`) or one pass produced no `*-report-full.json`. | +| `--switch-under-test is mutually exclusive with --baseline-version and --baseline-source-ref` | A switch experiment was combined with a source baseline. The experiment pipeline never does this; if you are invoking the scripts directly, clear the baseline selector — varying source and config at once makes the delta unattributable. | +| Switch experiment shows a ~0% delta everywhere | Expected for benchmarks the switch does not touch. If *every* benchmark is flat, check the run log's `Switch A/B` line actually names the switch, and that the switch is one the driver reads at startup via the runner config. | | Baseline source ref not found (PR pipeline) | `baselineSourceRef` is not a branch on `origin`, and the fallback `git clone --branch ` of `baselineRepoUrl` also failed (ref does not exist there, or the VM has no outbound access to the remote). The reason git gave is echoed into the build log and saved to `/diagnostics/git-*.log`. | | `Fetching baseline ref ... from the checkout's origin` is the last line, then the job stalls | Should no longer happen. The checkout is copied to the VM without credentials (ADO's checkout task defaults to `persistCredentials: false`), so a fetch from an authenticated `origin` used to sit on a `Username for ...` prompt forever. All network git calls now run with `GIT_TERMINAL_PROMPT=0`, no credential helper, stdin closed, and a `GIT_NET_TIMEOUT_SECS` (default 300s) hard timeout, so this fails in under a second and falls back to cloning `baselineRepoUrl`. A fetch failure here is expected and harmless on ADO. | | Ingestion step skipped | `enableKustoIngestion` is `false`, or `KustoClusterUri`, `KustoDatabase` or `KustoServiceConnection` (from `ADX Cluster Variables`) is empty (expected until the cluster is provisioned). | diff --git a/eng/pipelines/perf/scripts/interleave_perf.py b/eng/pipelines/perf/scripts/interleave_perf.py index 167002fb8f..fdf66a5913 100644 --- a/eng/pipelines/perf/scripts/interleave_perf.py +++ b/eng/pipelines/perf/scripts/interleave_perf.py @@ -101,17 +101,22 @@ def apply_affinity(proc, cpus): # -------------------------------------------------------------------------------------------------- # Running one unit and collecting its artifacts. # -------------------------------------------------------------------------------------------------- -def run_unit_process(exe_dir, assembly, unit, cwd, cpus, log_path): +def run_unit_process(exe_dir, assembly, unit, cwd, cpus, log_path, env_overrides=None): """Run one benchmark *unit* from the build at *exe_dir* in *cwd*. Returns the subprocess return code. Kept as a small seam so tests can substitute - a fake runner. + a fake runner. *env_overrides*, when given, is applied on top of the inherited + environment (e.g. a per-variant RUNNER_CONFIG so baseline and current can run with + different SqlClient behaviour flags, such as comparing the legacy vs new connection + pool from the SAME build). """ os.makedirs(cwd, exist_ok=True) cmd = ["dotnet", os.path.join(exe_dir, assembly)] env = dict(os.environ) env["PERF_BENCHMARK"] = unit env.pop("PERF_LIST_BENCHMARKS", None) + if env_overrides: + env.update(env_overrides) with open(log_path, "w", encoding="utf-8") as log: proc = subprocess.Popen(cmd, cwd=cwd, env=env, stdout=log, @@ -154,12 +159,18 @@ def collect_results(cwd, dest): class Runner: """Holds the invariant run parameters and performs interleaved unit passes.""" - def __init__(self, baseline_dir, current_dir, assembly, work_dir, cpus): + def __init__(self, baseline_dir, current_dir, assembly, work_dir, cpus, + baseline_runner_config=None, current_runner_config=None): self.baseline_dir = baseline_dir self.current_dir = current_dir self.assembly = assembly self.work_dir = work_dir self.cpus = cpus + # Optional per-variant RUNNER_CONFIG override (e.g. --switch-under-test needs baseline + # and current to run with different SqlClient behaviour flags even though they share the + # same build). None means "no override" -> both variants use the ambient RUNNER_CONFIG. + self.baseline_runner_config = baseline_runner_config + self.current_runner_config = current_runner_config def list_units(self): cmd = ["dotnet", os.path.join(self.current_dir, self.assembly)] @@ -175,7 +186,11 @@ def _run_one(self, variant, exe_dir, unit, rep, agg_dir): shutil.rmtree(cwd) os.makedirs(cwd, exist_ok=True) log_path = os.path.join(cwd, "run.log") - rc = run_unit_process(exe_dir, self.assembly, unit, cwd, self.cpus, log_path) + runner_config = (self.baseline_runner_config if variant == "baseline" + else self.current_runner_config) + env_overrides = {"RUNNER_CONFIG": runner_config} if runner_config else None + rc = run_unit_process(exe_dir, self.assembly, unit, cwd, self.cpus, log_path, + env_overrides=env_overrides) if rc != 0: _tail(log_path) raise RuntimeError(f"benchmark unit '{unit}' ({variant}, rep {rep}) failed (exit {rc}).") @@ -279,7 +294,6 @@ def orchestrate(runner, units, results_dir, threshold, reps): unconfirmed = [e for e in entries if e["status"] == "regression-unconfirmed"] return entries, confirmed, unconfirmed - # -------------------------------------------------------------------------------------------------- # Rendering. # -------------------------------------------------------------------------------------------------- @@ -332,6 +346,7 @@ def render_markdown(entries, confirmed, unconfirmed, baseline_version, threshold ) ) lines.append("") + return "\n".join(lines) @@ -353,6 +368,14 @@ def main(argv=None): parser.add_argument("--reps", type=int, default=3, help="Total interleaved passes for a flagged unit (best-of-N). 1 disables confirmation.") parser.add_argument("--baseline-version", default="baseline") + parser.add_argument("--baseline-runner-config", default=None, + help="Override RUNNER_CONFIG for baseline-variant subprocesses only " + "(e.g. to force a different SqlClient behaviour flag for the " + "baseline, such as comparing the legacy vs new connection pool " + "from the same build). Omit to use the ambient RUNNER_CONFIG for " + "both variants (default behaviour).") + parser.add_argument("--current-runner-config", default=None, + help="Override RUNNER_CONFIG for current-variant subprocesses only.") parser.add_argument("--client-cpus", default=os.environ.get("PERF_CLIENT_CPUS", ""), help="CPU set to pin the benchmark client to, e.g. '16-31'.") parser.add_argument("--fail-on-regression", action="store_true", @@ -376,7 +399,9 @@ def main(argv=None): runner = Runner(os.path.abspath(args.baseline_exe_dir), os.path.abspath(args.current_exe_dir), - args.assembly, work_dir, cpus) + args.assembly, work_dir, cpus, + baseline_runner_config=args.baseline_runner_config, + current_runner_config=args.current_runner_config) units = runner.list_units() if not units: @@ -389,7 +414,8 @@ def main(argv=None): # Outputs. comparison_dir = os.path.join(results_dir, "comparison") - md = render_markdown(entries, confirmed, unconfirmed, args.baseline_version, args.threshold, args.reps) + md = render_markdown(entries, confirmed, unconfirmed, + args.baseline_version, args.threshold, args.reps) with open(os.path.join(comparison_dir, "comparison.md"), "w", encoding="utf-8") as fh: fh.write(md + "\n") with open(os.path.join(comparison_dir, "comparison.json"), "w", encoding="utf-8") as fh: diff --git a/eng/pipelines/perf/scripts/run-perf-tests.ps1 b/eng/pipelines/perf/scripts/run-perf-tests.ps1 index c91239eb80..aad97b5b60 100644 --- a/eng/pipelines/perf/scripts/run-perf-tests.ps1 +++ b/eng/pipelines/perf/scripts/run-perf-tests.ps1 @@ -54,7 +54,17 @@ param( [ValidateSet("", "true", "false")] [string]$UseOptimizedAsyncBehaviour = "", [ValidateSet("", "true", "false")] - [string]$UseConnectionPoolV2 = "" + [string]$UseConnectionPoolV2 = "", + # Alternative to -BaselineVersion/-BaselineSourceRef: an A/B experiment on ONE runner-config + # switch. Both passes build the SAME source; only the named switch differs (baseline=false, + # current=true), which is the only way to compare a switch whose value is latched process-wide + # (e.g. UseConnectionPoolV2 is read and cached the first time a pool is created). Mutually + # exclusive with the other two baseline selectors, and overrides the matching -Use* flag (which + # would otherwise be ambiguous: one value cannot describe two passes). ValidateSet restricts it + # to switches this script knows how to stamp, so a typo fails fast at binding time instead of + # silently writing an inert key and reporting a meaningless zero-delta comparison. + [ValidateSet("", "UseConnectionPoolV2", "UseOptimizedAsyncBehaviour", "UseManagedSniOnWindows")] + [string]$SwitchUnderTest = "" ) $ErrorActionPreference = "Stop" @@ -128,6 +138,7 @@ Write-Host " Results dir : $ResultsDir" Write-Host " Run mode : $RunMode (confirmation runs: $ConfirmationRuns)" Write-Host " Baseline ver : $(if ($BaselineVersion) { $BaselineVersion } else { '' })" Write-Host " Baseline ref : $(if ($BaselineSourceRef) { $BaselineSourceRef } else { '' })" +Write-Host " Switch A/B : $(if ($SwitchUnderTest) { "$SwitchUnderTest (baseline=false vs current=true)" } else { '' })" Write-Host " SQL_SERVER : $SqlServer" Write-Host " PERF_CLIENT_CPUS: $($env:PERF_CLIENT_CPUS)" Write-Host " PERF_SQL_CPUS : $($env:PERF_SQL_CPUS)" @@ -141,6 +152,21 @@ if (-not (Test-Path $PerfProject)) { if ((-not [string]::IsNullOrEmpty($BaselineVersion)) -and (-not [string]::IsNullOrEmpty($BaselineSourceRef))) { throw "-BaselineVersion and -BaselineSourceRef are mutually exclusive." } +if ((-not [string]::IsNullOrEmpty($SwitchUnderTest)) -and ((-not [string]::IsNullOrEmpty($BaselineVersion)) -or (-not [string]::IsNullOrEmpty($BaselineSourceRef)))) { + throw "-SwitchUnderTest is mutually exclusive with -BaselineVersion and -BaselineSourceRef: it compares the SAME source build with one switch flipped, so mixing in a source change would make the delta unattributable." +} +# -SwitchUnderTest forces its switch explicitly for each pass (baseline=false, current=true), so a +# separately-supplied -Use* flag for that SAME switch would be silently overridden; warn rather than +# let that go unnoticed. Other -Use* flags still apply normally to both passes. +$conflictingFlagValue = switch ($SwitchUnderTest) { + "UseConnectionPoolV2" { $UseConnectionPoolV2 } + "UseOptimizedAsyncBehaviour" { $UseOptimizedAsyncBehaviour } + "UseManagedSniOnWindows" { $UseManagedSniOnWindows } + default { "" } +} +if (-not [string]::IsNullOrEmpty($conflictingFlagValue)) { + Write-Warning "-$SwitchUnderTest is ignored when -SwitchUnderTest is $SwitchUnderTest (baseline forces false, current forces true)." +} if ([string]::IsNullOrEmpty($SqlPassword)) { throw "SQL_PASSWORD environment variable is not set (expected from the perf template)." } @@ -244,14 +270,93 @@ if ($sqlcmd) { # # The Perf Test Lab already provides the isolated dedicated host, the tuned SQL instance and the # disjoint client CPU set (PERF_CLIENT_CPUS, pinned per pass below). These are the remaining -# harness-owned controls: per-run diagnostics, a fail-loud preflight and a warm-up, so a run's -# mean/variance is steadier and a broken run cannot masquerade as a pass. (The glibc allocator and -# sysctl tuning from the Linux harness are Linux-only and intentionally omitted here.) +# harness-owned controls: network tuning, per-run diagnostics, a fail-loud preflight and a warm-up, +# so a run's mean/variance is steadier and a broken run cannot masquerade as a pass. (The glibc +# allocator tuning from the Linux harness is genuinely glibc-specific and is omitted here; the +# ephemeral-port tuning is NOT Linux-specific and has a Windows equivalent -- see §2.9 below.) #################################################################################################### $DiagDir = Join-Path $ResultsDir "diagnostics" New-Item -ItemType Directory -Force -Path $DiagDir | Out-Null +# --- §2.9 Network tuning (best-effort; needs elevation, so it must never fail the run) ------------ +# Connection-churn benches exhaust ephemeral ports, and the churn is far heavier than it looks: +# SqlConnectionRunner has [Params(true, false)] on Pooling, so its Pooling=False cases open a fresh +# physical TCP connection for every single operation -- 4 cases x (5 warmup + 50 iterations) x 30 +# invocations = ~6,600 back-to-back connects with no delay between them. ConnectionPoolRampRunner +# adds ~2,700 more (its [IterationSetup] calls ClearAllPools(), so every iteration ramps from cold) +# and ConnectionPoolStressRunner ~3,100 (MinPoolSize = MaxPoolSize, so all 42 cases fill a fresh +# pool). Every one of those closed sockets then sits in TIME_WAIT holding its port. +# +# Windows defaults to 16,384 dynamic ports (49152-65535) with a 120s TcpTimedWaitDelay, i.e. only +# ~136 sustained connects/sec. The suite wants orders of magnitude more, so without this the client +# drains the port pool within seconds and every subsequent connect blocks waiting for a socket to +# leave TIME_WAIT. That inflates socket setup latency enormously -- and because runnerconfig.jsonc +# pins InvocationCount for every runner, BenchmarkDotNet's pilot stage never runs and the stall is +# not normalised away; it lands directly in the reported per-operation numbers. The Linux harness +# fixes this with ip_local_port_range + tcp_tw_reuse (run-perf-tests.sh §2.9); this is the Windows +# equivalent, so the two platforms measure the driver rather than their own port pressure. +# +# The range starts at 10000 rather than the Linux harness's 1024: on Windows the dynamic range is +# also the range the stack allocates from for any bind(0), and reaching down into the registered +# range risks colliding with a service that expects to bind a fixed port (SQL Server's own 1433 +# among them, when the client and server share a host). 10000-65535 still yields 55,536 ports, +# ~3.4x the default, which is comfortably past what the suite consumes. +# +# netsh is invoked with the preference relaxed for the duration of the call: on Windows PowerShell +# 5.1 a non-elevated netsh writes "requires elevation" to stderr, which $ErrorActionPreference='Stop' +# would otherwise promote to a terminating error (see "Native-command error handling" above). This +# tuning is best-effort by design, so it reports and continues rather than failing the run. +$previousPreference = $ErrorActionPreference +$ErrorActionPreference = 'Continue' +try { + $portsBefore = (netsh int ipv4 show dynamicport tcp 2>&1 | Out-String).Trim() + $LASTEXITCODE = 0 + netsh int ipv4 set dynamicport tcp start=10000 num=55536 2>&1 | Out-Null + if ($LASTEXITCODE -eq 0) { + Write-Host "Widened the TCP dynamic port range to 10000-65535 (55,536 ephemeral ports)." + } else { + Write-Warning ("Could not widen the TCP dynamic port range (netsh exit $LASTEXITCODE); " + + "connection-churn benchmarks may stall on TIME_WAIT and the run's " + + "socket-setup latency will be inflated. Is the agent elevated?") + } + $portsAfter = (netsh int ipv4 show dynamicport tcp 2>&1 | Out-String).Trim() + "=== before ===`n$portsBefore`n`n=== after ===`n$portsAfter" | + Out-File -FilePath (Join-Path $DiagDir "tcp-dynamicport.txt") -Encoding UTF8 +} catch { + Write-Warning "Could not tune the TCP dynamic port range: $_" +} finally { + $ErrorActionPreference = $previousPreference +} + +# TcpTimedWaitDelay (seconds a closed socket lingers in TIME_WAIT; valid range 30-300, default 120) +# has no immediate-effect equivalent to Linux's tcp_tw_reuse: the stack reads it at boot, so writing +# it here could not help the current run. It is also a persistent, machine-wide change, which is a +# wider blast radius than the Linux harness's 'sysctl -w' (that resets at reboot). So report it +# rather than set it, and leave the decision to VM provisioning. +try { + $tcpParams = 'HKLM:\SYSTEM\CurrentControlSet\Services\Tcpip\Parameters' + $tcpProps = Get-ItemProperty -Path $tcpParams -ErrorAction SilentlyContinue + # StrictMode makes a missing-property access a terminating error, so probe before reading. + $effective = if ($null -ne $tcpProps -and + ($tcpProps.PSObject.Properties.Name -contains 'TcpTimedWaitDelay')) { + $tcpProps.TcpTimedWaitDelay + } else { + $null + } + $effectiveText = if ($null -ne $effective) { "$effective s" } else { "unset (stack default, 120 s)" } + "TcpTimedWaitDelay in effect for this run: $effectiveText" | + Out-File -FilePath (Join-Path $DiagDir "tcp-timedwaitdelay.txt") -Encoding UTF8 + if ($null -eq $effective -or $effective -gt 60) { + Write-Warning ("TcpTimedWaitDelay is $effectiveText. Closed sockets hold their port for " + + "that long, which caps sustained connects even with the widened range. " + + "To lower it, set HKLM:\SYSTEM\CurrentControlSet\Services\Tcpip\Parameters" + + "\TcpTimedWaitDelay (DWORD) to 30 at VM provisioning; it takes effect on reboot.") + } +} catch { + Write-Warning "Could not read TcpTimedWaitDelay: $_" +} + # --- §2.11 Capture host CPU topology (static, once per run) --------------------------------------- try { Get-CimInstance Win32_Processor | @@ -302,20 +407,11 @@ $env:RUNNER_CONFIG = $RunnerConfig # It needs no per-run modification, so point the env var at the checked-in file directly. $env:DATATYPES_CONFIG = Join-Path $PerfDir "datatypes.json" -$srcConfig = Join-Path $PerfDir "runnerconfig.jsonc" -$rawConfig = Get-Content $srcConfig -Raw -# Strip // line comments so ConvertFrom-Json accepts the .jsonc content. -$rawConfig = ($rawConfig -split "`n" | ForEach-Object { $_ -replace '(?m)^\s*//.*$', '' }) -join "`n" -$cfg = ConvertFrom-Json $rawConfig - # SqlClient connection-string values may be wrapped in double quotes; doubling any embedded double # quote lets a password containing ';', '=', spaces or single quotes be parsed as a single literal # value instead of corrupting the connection string. $escapedPassword = '"' + ($SqlPassword -replace '"', '""') + '"' -$cfg.ConnectionString = "Server=tcp:$SqlServer,1433;User ID=sa;Password=$escapedPassword;Initial Catalog=$DbName;TrustServerCertificate=True;Encrypt=False;" -# Apply the optional SqlClient behaviour overrides supplied by the pipeline. An empty value leaves -# the checked-in default untouched; otherwise the flag is forced to the requested boolean so the -# benchmarks run with (and PerfRun.Config records) exactly the requested behaviour. + function Set-CfgBool { param($Config, [string]$Name, [string]$Value) if (-not [string]::IsNullOrEmpty($Value)) { @@ -324,11 +420,51 @@ function Set-CfgBool { else { $Config | Add-Member -NotePropertyName $Name -NotePropertyValue $b } } } -Set-CfgBool $cfg "UseManagedSniOnWindows" $UseManagedSniOnWindows -Set-CfgBool $cfg "UseOptimizedAsyncBehaviour" $UseOptimizedAsyncBehaviour -Set-CfgBool $cfg "UseConnectionPoolV2" $UseConnectionPoolV2 -$cfg | ConvertTo-Json -Depth 10 | Set-Content -Path $RunnerConfig -Encoding UTF8 -Write-Host "Wrote runner config to $RunnerConfig (Server=tcp:$SqlServer,1433; Initial Catalog=$DbName)" + +# Write-RunnerConfig [switchName] [switchValue] +# Writes one runner config (checked-in runnerconfig.jsonc + injected connection string + behaviour +# overrides) to . When is given, that config key is forced to +# ("true"/"false") regardless of the corresponding -Use* parameter -- used by -SwitchUnderTest, which +# needs a different value for the same switch in each pass. With no switch name the config is built +# purely from the -Use* parameters, exactly as before. +function Write-RunnerConfig { + param([string]$Dst, [string]$SwitchName = "", [string]$SwitchValue = "") + $srcConfig = Join-Path $PerfDir "runnerconfig.jsonc" + $rawConfig = Get-Content $srcConfig -Raw + # Strip // line comments so ConvertFrom-Json accepts the .jsonc content. + $rawConfig = ($rawConfig -split "`n" | ForEach-Object { $_ -replace '(?m)^\s*//.*$', '' }) -join "`n" + $cfg = ConvertFrom-Json $rawConfig + + $cfg.ConnectionString = "Server=tcp:$SqlServer,1433;User ID=sa;Password=$escapedPassword;Initial Catalog=$DbName;TrustServerCertificate=True;Encrypt=False;" + # Apply the optional SqlClient behaviour overrides supplied by the pipeline. An empty value + # leaves the checked-in default untouched; otherwise the flag is forced to the requested boolean + # so the benchmarks run with (and PerfRun.Config records) exactly the requested behaviour. The + # switch-under-test override (when this config names one) takes precedence over the matching + # -Use* parameter, so a single checked-in template can be stamped out per pass with that one + # switch flipped and everything else identical. + $sniValue = if ($SwitchName -eq "UseManagedSniOnWindows") { $SwitchValue } else { $UseManagedSniOnWindows } + $asyncValue = if ($SwitchName -eq "UseOptimizedAsyncBehaviour") { $SwitchValue } else { $UseOptimizedAsyncBehaviour } + $poolValue = if ($SwitchName -eq "UseConnectionPoolV2") { $SwitchValue } else { $UseConnectionPoolV2 } + Set-CfgBool $cfg "UseManagedSniOnWindows" $sniValue + Set-CfgBool $cfg "UseOptimizedAsyncBehaviour" $asyncValue + Set-CfgBool $cfg "UseConnectionPoolV2" $poolValue + $cfg | ConvertTo-Json -Depth 10 | Set-Content -Path $Dst -Encoding UTF8 + Write-Host "Wrote runner config to $Dst (Server=tcp:$SqlServer,1433; Initial Catalog=$DbName)" +} + +Write-RunnerConfig -Dst $RunnerConfig + +# -SwitchUnderTest needs two DIFFERENT runner configs (baseline runs the switch off, current runs it +# on), so stamp out two more copies here alongside the shared one above. Everything else in them is +# identical, so any measured delta is attributable to the switch alone. +$BaselineRunnerConfig = "" +$CurrentRunnerConfig = "" +if (-not [string]::IsNullOrEmpty($SwitchUnderTest)) { + $BaselineRunnerConfig = Join-Path $RepoRoot "perf-runnerconfig-baseline.json" + $CurrentRunnerConfig = Join-Path $RepoRoot "perf-runnerconfig-current.json" + Write-RunnerConfig -Dst $BaselineRunnerConfig -SwitchName $SwitchUnderTest -SwitchValue "false" + Write-RunnerConfig -Dst $CurrentRunnerConfig -SwitchName $SwitchUnderTest -SwitchValue "true" +} #################################################################################################### # 4 & 5. Run the benchmarks, pinned to the reserved client CPU set. @@ -688,6 +824,11 @@ if (-not [string]::IsNullOrEmpty($BaselineVersion)) { $baselineSource = Initialize-BaselineSource -Ref $BaselineSourceRef $BaselineLabel = $baselineSource.Label $BaselineProject = $baselineSource.Project +} elseif (-not [string]::IsNullOrEmpty($SwitchUnderTest)) { + # Switch A/B: SAME source/project for both passes ($BaselineProject/$BaselineBuildArgs are + # already the candidate's, set above), so only the runner config differs (see + # $BaselineRunnerConfig/$CurrentRunnerConfig written above: the named switch off vs on). + $BaselineLabel = "$SwitchUnderTest=false" } # Record the resolved baseline label (for a source baseline this is '@') in the results @@ -704,8 +845,17 @@ if ((-not [string]::IsNullOrEmpty($BaselineLabel)) -and ($RunMode -eq "interleav # orchestrator run one unit at a time (baseline then candidate) and confirm any flagged # regression across N passes before it counts toward the gate. #################################################################################################### - $baselineExeDir = Build-Variant "baseline" $BaselineProject $BaselineBuildArgs - $currentExeDir = Build-Variant "current" $PerfProject @() + if (-not [string]::IsNullOrEmpty($SwitchUnderTest)) { + # Switch A/B measures one build against itself with a switch flipped, so building the same + # project twice would just burn several minutes producing identical bits. Build once and + # point both variants at it; the orchestrator runs each variant in its own working directory + # (rep//), so a shared exe dir cannot cross-contaminate their artifacts. + $currentExeDir = Build-Variant "current" $PerfProject @() + $baselineExeDir = $currentExeDir + } else { + $baselineExeDir = Build-Variant "baseline" $BaselineProject $BaselineBuildArgs + $currentExeDir = Build-Variant "current" $PerfProject @() + } $interleaveArgs = @( "--baseline-exe-dir", $baselineExeDir, @@ -717,6 +867,12 @@ if ((-not [string]::IsNullOrEmpty($BaselineLabel)) -and ($RunMode -eq "interleav "--baseline-version", $BaselineLabel, "--client-cpus", "$($env:PERF_CLIENT_CPUS)" ) + # -SwitchUnderTest: baseline and current subprocesses need DIFFERENT RUNNER_CONFIG values (the + # switch off vs on), even though both are otherwise the same build/env; every other baseline + # flavour keeps sharing the single ambient RUNNER_CONFIG set above. + if (-not [string]::IsNullOrEmpty($SwitchUnderTest)) { + $interleaveArgs += @("--baseline-runner-config", $BaselineRunnerConfig, "--current-runner-config", $CurrentRunnerConfig) + } if ($FailOnRegression) { Write-Host "Regression gate ENABLED: a CONFIRMED candidate-slower regression (> $RegressionThreshold%) will fail the run." $interleaveArgs += "--fail-on-regression" @@ -726,7 +882,11 @@ if ((-not [string]::IsNullOrEmpty($BaselineLabel)) -and ($RunMode -eq "interleav } elseif (-not [string]::IsNullOrEmpty($BaselineLabel)) { # --- Legacy sequential path: full baseline pass, then full candidate pass, then compare ------- + # -SwitchUnderTest needs a different RUNNER_CONFIG per pass; every other baseline flavour + # keeps using the single ambient RUNNER_CONFIG set above (unchanged behaviour). + 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 @() Write-Host "Comparing current branch against baseline $BaselineLabel ..." diff --git a/eng/pipelines/perf/scripts/run-perf-tests.sh b/eng/pipelines/perf/scripts/run-perf-tests.sh index 8d05340342..3373d9a0a2 100755 --- a/eng/pipelines/perf/scripts/run-perf-tests.sh +++ b/eng/pipelines/perf/scripts/run-perf-tests.sh @@ -65,10 +65,22 @@ confirmationRuns="3" useManagedSniOnWindows="" useOptimizedAsyncBehaviour="" useConnectionPoolV2="" +# Alternative to --baseline-version/--baseline-source-ref: an A/B experiment on ONE runner-config +# switch. Both passes build the SAME source; only the named switch differs (baseline=false, +# current=true), which is the only way to compare a switch whose value is latched process-wide (e.g. +# UseConnectionPoolV2 is read and cached the first time a pool is created). Mutually exclusive with +# the other two baseline selectors, and overrides the matching --use-* flag (which would otherwise be +# ambiguous: one value cannot describe two passes). +switchUnderTest="" +# Runner-config switches this script is allowed to A/B. Restricted to a known list so a typo fails +# fast here instead of silently writing an inert key into the runner config and reporting a +# meaningless zero-delta comparison. +SUPPORTED_SWITCHES=("UseConnectionPoolV2" "UseOptimizedAsyncBehaviour" "UseManagedSniOnWindows") usage() { echo "Usage: $0 [--configuration ] [--framework ] [--results-subdir ]" \ - "[--baseline-version | --baseline-source-ref [--baseline-repo-url ]]" \ + "[--baseline-version | --baseline-source-ref [--baseline-repo-url ] |" \ + "--switch-under-test <${SUPPORTED_SWITCHES[*]}>]" \ "[--regression-threshold ] [--fail-on-regression]" \ "[--run-mode interleaved|sequential] [--confirmation-runs ]" \ "[--use-managed-sni-on-windows true|false] [--use-optimized-async-behaviour true|false]" \ @@ -83,6 +95,7 @@ while [[ $# -gt 0 ]]; do --baseline-version) baselineVersion="$2"; shift 2 ;; --baseline-source-ref) baselineSourceRef="$2"; shift 2 ;; --baseline-repo-url) baselineRepoUrl="$2"; shift 2 ;; + --switch-under-test) switchUnderTest="$2"; shift 2 ;; --regression-threshold) regressionThreshold="$2"; shift 2 ;; --fail-on-regression) failOnRegression="true"; shift 1 ;; --run-mode) runMode="$2"; shift 2 ;; @@ -103,12 +116,26 @@ case "${runMode}" in *) echo "ERROR: --run-mode must be 'interleaved' or 'sequential' (got '${runMode}')." >&2 usage; exit 2 ;; esac -# 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. +# The three baseline selectors describe different builds/configs of the same "baseline" pass, so +# requesting more than one is always a mistake; fail fast rather than silently honouring one of them. if [[ -n "${baselineVersion}" && -n "${baselineSourceRef}" ]]; then echo "ERROR: --baseline-version and --baseline-source-ref are mutually exclusive." >&2 usage; exit 2 fi +if [[ -n "${switchUnderTest}" && ( -n "${baselineVersion}" || -n "${baselineSourceRef}" ) ]]; then + echo "ERROR: --switch-under-test is mutually exclusive with --baseline-version and --baseline-source-ref: it compares the SAME source build with one switch flipped, so mixing in a source change would make the delta unattributable." >&2 + usage; exit 2 +fi +if [[ -n "${switchUnderTest}" ]]; then + switchSupported="false" + for supported in "${SUPPORTED_SWITCHES[@]}"; do + [[ "${switchUnderTest}" == "${supported}" ]] && switchSupported="true" + done + if [[ "${switchSupported}" != "true" ]]; then + echo "ERROR: --switch-under-test must be one of: ${SUPPORTED_SWITCHES[*]} (got '${switchUnderTest}')." >&2 + usage; exit 2 + fi +fi if ! [[ "${confirmationRuns}" =~ ^[0-9]+$ ]] || [[ "${confirmationRuns}" -lt 1 ]]; then echo "ERROR: --confirmation-runs must be a positive integer (got '${confirmationRuns}')." >&2 usage; exit 2 @@ -123,6 +150,18 @@ validate_bool() { # $1 = flag name (for the message), $2 = value validate_bool use-managed-sni-on-windows "${useManagedSniOnWindows}" validate_bool use-optimized-async-behaviour "${useOptimizedAsyncBehaviour}" validate_bool use-connection-pool-v2 "${useConnectionPoolV2}" +# --switch-under-test forces its switch explicitly for each pass (baseline=false, current=true), so a +# separately-supplied --use-* flag for that SAME switch would be silently overridden; warn rather +# than let that go unnoticed. Other --use-* flags still apply normally to both passes. +case "${switchUnderTest}" in + UseConnectionPoolV2) conflictingValue="${useConnectionPoolV2}"; conflictingFlag="--use-connection-pool-v2" ;; + UseOptimizedAsyncBehaviour) conflictingValue="${useOptimizedAsyncBehaviour}"; conflictingFlag="--use-optimized-async-behaviour" ;; + UseManagedSniOnWindows) conflictingValue="${useManagedSniOnWindows}"; conflictingFlag="--use-managed-sni-on-windows" ;; + *) conflictingValue=""; conflictingFlag="" ;; +esac +if [[ -n "${conflictingValue}" ]]; then + echo "WARNING: ${conflictingFlag} is ignored when --switch-under-test is ${switchUnderTest} (baseline forces false, current forces true)." >&2 +fi #################################################################################################### # Resolve paths @@ -148,6 +187,7 @@ echo " Framework : ${framework}" echo " Results dir : ${RESULTS_DIR}" echo " Baseline ver : ${baselineVersion:-}" echo " Baseline ref : ${baselineSourceRef:-}" +echo " Switch A/B : ${switchUnderTest:-}${switchUnderTest:+ (baseline=false vs current=true)}" echo " Run mode : ${runMode} (confirmation runs: ${confirmationRuns})" echo " SQL_SERVER : ${SQL_SERVER:-}" echo " PERF_CLIENT_CPUS: ${PERF_CLIENT_CPUS:-}" @@ -295,7 +335,9 @@ export MALLOC_MMAP_THRESHOLD_="${MALLOC_MMAP_THRESHOLD_:-134217728}" # 128 MiB export MALLOC_TRIM_THRESHOLD_="${MALLOC_TRIM_THRESHOLD_:--1}" # never trim # --- §2.9 Network tuning (best-effort; needs privilege, so it must never fail the run) ------------ -# Connection-churn benches (ConnectionPoolStress, ParallelAsyncConnection) exhaust ephemeral ports; +# Connection-churn benches (ConnectionPoolStress, ConnectionPoolRamp, +# ConnectionPoolThreadPoolPressure, ParallelAsyncConnection) +# exhaust ephemeral ports; # widen the range and allow TIME_WAIT reuse so socket setup latency stays stable. 'sudo -n' keeps # this non-interactive: on a VM without passwordless sudo it fails immediately instead of blocking # on a password prompt, then we fall back to a non-sudo sysctl (and finally give up quietly). @@ -358,10 +400,20 @@ export PERF_CFG_USE_MANAGED_SNI="${useManagedSniOnWindows}" export PERF_CFG_USE_OPTIMIZED_ASYNC="${useOptimizedAsyncBehaviour}" export PERF_CFG_USE_CONNECTION_POOL_V2="${useConnectionPoolV2}" -python3 - "$PERF_DIR/runnerconfig.jsonc" "$RUNNER_CONFIG" <<'PY' +# write_runner_config [switch_name] [switch_value] +# Writes one runner config (checked-in runnerconfig.jsonc + injected connection string + behaviour +# overrides) to . When is given, that config key is forced to +# ("true"/"false") regardless of the corresponding PERF_CFG_* value -- used by --switch-under-test, +# which needs a different value for the same switch in each pass. With no switch name the config is +# built purely from the PERF_CFG_* values, exactly as before. +write_runner_config() { + local dst="$1" + local switch_name="${2:-}" + local switch_value="${3:-}" + python3 - "$PERF_DIR/runnerconfig.jsonc" "$dst" "$switch_name" "$switch_value" <<'PY' import json, os, re, sys -src, dst = sys.argv[1], sys.argv[2] +src, dst, switch_name, switch_value = sys.argv[1], sys.argv[2], sys.argv[3], sys.argv[4] with open(src, "r", encoding="utf-8-sig") as fh: text = fh.read() @@ -387,13 +439,16 @@ cfg["ConnectionString"] = ( # Apply the optional SqlClient behaviour overrides supplied by the pipeline. An empty value leaves # the checked-in default untouched; otherwise the flag is forced to the requested boolean so the -# benchmarks run with (and PerfRun.Config records) exactly the requested behaviour. +# benchmarks run with (and PerfRun.Config records) exactly the requested behaviour. The +# switch-under-test override (when this config names one) takes precedence over the corresponding +# PERF_CFG_* value, so a single checked-in template can be stamped out per pass with that one switch +# flipped and everything else identical. for env_name, cfg_key in ( ("PERF_CFG_USE_MANAGED_SNI", "UseManagedSniOnWindows"), ("PERF_CFG_USE_OPTIMIZED_ASYNC", "UseOptimizedAsyncBehaviour"), ("PERF_CFG_USE_CONNECTION_POOL_V2", "UseConnectionPoolV2"), ): - val = os.environ.get(env_name, "") + val = switch_value if cfg_key == switch_name else os.environ.get(env_name, "") if val != "": cfg[cfg_key] = (val.lower() == "true") @@ -402,6 +457,21 @@ with open(dst, "w", encoding="utf-8") as fh: print(f"Wrote runner config to {dst} (Server=tcp:{server},1433; Initial Catalog={db})") PY +} + +write_runner_config "${RUNNER_CONFIG}" + +# --switch-under-test needs two DIFFERENT runner configs (baseline runs the switch off, current runs +# it on), so stamp out two more copies here alongside the shared one above. Everything else in them +# is identical, so any measured delta is attributable to the switch alone. +BASELINE_RUNNER_CONFIG="" +CURRENT_RUNNER_CONFIG="" +if [[ -n "${switchUnderTest}" ]]; then + BASELINE_RUNNER_CONFIG="${REPO_ROOT}/perf-runnerconfig-baseline.json" + CURRENT_RUNNER_CONFIG="${REPO_ROOT}/perf-runnerconfig-current.json" + write_runner_config "${BASELINE_RUNNER_CONFIG}" "${switchUnderTest}" "false" + write_runner_config "${CURRENT_RUNNER_CONFIG}" "${switchUnderTest}" "true" +fi #################################################################################################### # 4 & 5. Run the benchmarks, pinned to the reserved client CPU set. @@ -686,6 +756,11 @@ elif [[ -n "${baselineSourceRef}" ]]; then prepare_baseline_source "${baselineSourceRef}" baselineLabel="${BASELINE_SRC_LABEL}" baselineProject="${BASELINE_PERF_PROJECT}" +elif [[ -n "${switchUnderTest}" ]]; then + # Switch A/B: SAME source/project for both passes (baselineProject/baselineBuildArgs are already + # the candidate's, set above), so only the runner config differs (see BASELINE_RUNNER_CONFIG / + # CURRENT_RUNNER_CONFIG written above: the named switch off vs on). + baselineLabel="${switchUnderTest}=false" fi # Record the resolved baseline label (for a source baseline this is '@') in the results @@ -702,12 +777,24 @@ if [[ -n "${baselineLabel}" && "${runMode}" == "interleaved" ]]; then # orchestrator run one unit at a time (baseline then candidate) and confirm any flagged # regression across N passes before it counts toward the gate. ################################################################################################ - build_variant "baseline" "${baselineProject}" ${baselineBuildArgs[@]+"${baselineBuildArgs[@]}"} - build_variant "current" "${PERF_PROJECT}" + if [[ -n "${switchUnderTest}" ]]; then + # Switch A/B measures one build against itself with a switch flipped, so building the same + # project twice would just burn several minutes producing identical bits. Build once and + # point both variants at it; the orchestrator runs each variant in its own working directory + # (rep//), so a shared exe dir cannot cross-contaminate their artifacts. + build_variant "current" "${PERF_PROJECT}" + baselineExeDir="${REPO_ROOT}/perf-build-current" + currentExeDir="${REPO_ROOT}/perf-build-current" + else + build_variant "baseline" "${baselineProject}" ${baselineBuildArgs[@]+"${baselineBuildArgs[@]}"} + build_variant "current" "${PERF_PROJECT}" + baselineExeDir="${REPO_ROOT}/perf-build-baseline" + currentExeDir="${REPO_ROOT}/perf-build-current" + fi interleave_args=( - --baseline-exe-dir "${REPO_ROOT}/perf-build-baseline" - --current-exe-dir "${REPO_ROOT}/perf-build-current" + --baseline-exe-dir "${baselineExeDir}" + --current-exe-dir "${currentExeDir}" --assembly "PerformanceTests.dll" --results-dir "${RESULTS_DIR}" --threshold "${regressionThreshold}" @@ -715,6 +802,15 @@ if [[ -n "${baselineLabel}" && "${runMode}" == "interleaved" ]]; then --baseline-version "${baselineLabel}" --client-cpus "${PERF_CLIENT_CPUS:-}" ) + # --switch-under-test: baseline and current subprocesses need DIFFERENT RUNNER_CONFIG values (the + # switch off vs on), even though both are otherwise the same build/env; every other baseline + # flavour keeps sharing the single ambient RUNNER_CONFIG set above. + if [[ -n "${switchUnderTest}" ]]; then + interleave_args+=( + --baseline-runner-config "${BASELINE_RUNNER_CONFIG}" + --current-runner-config "${CURRENT_RUNNER_CONFIG}" + ) + fi if [[ "${failOnRegression}" == "true" ]]; then echo "Regression gate ENABLED: a CONFIRMED candidate-slower regression (> ${regressionThreshold}%) will fail the run." interleave_args+=(--fail-on-regression) @@ -724,7 +820,15 @@ if [[ -n "${baselineLabel}" && "${runMode}" == "interleaved" ]]; then elif [[ -n "${baselineLabel}" ]]; then # --- Legacy sequential path: full baseline pass, then full candidate pass, then compare ------- + # --switch-under-test needs a different RUNNER_CONFIG per pass; every other baseline flavour + # keeps using the single ambient RUNNER_CONFIG exported above (unchanged behaviour). + if [[ -n "${switchUnderTest}" ]]; then + export RUNNER_CONFIG="${BASELINE_RUNNER_CONFIG}" + fi run_pass "baseline" "${baselineProject}" ${baselineBuildArgs[@]+"${baselineBuildArgs[@]}"} + if [[ -n "${switchUnderTest}" ]]; then + export RUNNER_CONFIG="${CURRENT_RUNNER_CONFIG}" + fi run_pass "current" "${PERF_PROJECT}" echo "Comparing current branch against baseline ${baselineLabel} ..." diff --git a/eng/pipelines/perf/sqlclient-perf-experiment.yml b/eng/pipelines/perf/sqlclient-perf-experiment.yml new file mode 100644 index 0000000000..7539c97bfa --- /dev/null +++ b/eng/pipelines/perf/sqlclient-perf-experiment.yml @@ -0,0 +1,235 @@ +#################################################################################################### +# Licensed to the .NET Foundation under one or more agreements. The .NET Foundation licenses this +# file to you under the MIT license. See the LICENSE file in the project root for more information. +#################################################################################################### + +# SqlClient Switch Experiment Performance pipeline. +# +# Third sibling of sqlclient-perf-pipeline.yml and sqlclient-perf-pr-pipeline.yml. It runs the SAME +# benchmarks, on the SAME Perf Test Lab extends template (v1/Perf.Test.Job.yml@PerfTemplates), +# through the SAME on-VM scripts. What differs is the QUESTION it answers: +# +# * perf pipeline - "has this branch regressed against a released package?" (source varies) +# * perf-pr pipeline - "has this PR regressed the branch it merges into?" (source varies) +# * THIS pipeline - "what does this AppContext/runner switch cost or buy?" (CONFIG varies) +# +# Both passes here build the SAME source (whatever branch the run is queued on); only ONE runner +# config switch differs between them - baseline runs it OFF, current runs it ON. Two separate +# processes are required because these switches are latched process-wide (e.g. UseConnectionPoolV2 is +# read and cached the first time a connection pool is created), so they cannot be toggled between +# benchmarks inside a single run. Queue this pipeline on a PR branch to ask "what does the switch do +# to my change?", or on main to ask "what does the switch do to main?". +# +# * No Kusto - switch-experiment results are NEVER ingested into the perf database, and this is why +# the mode lives in its own pipeline rather than as a flag on the other two: +# - both PerfRun rows would share one DerivedRunId (driver|commit|pipelineRunId), +# because both passes are the same commit in the same pipeline run; +# - PerfRun.Config is stamped once per run, so the two rows could not record the +# differing switch values that are the entire point of the experiment; +# - nothing marks a row as an experiment, so the switch-ON pass would be +# indistinguishable from an ordinary measurement of the branch and would distort +# the very trends the other two pipelines exist to protect. +# Having no ADX variable group and no translate/ingest steps in this file makes that +# exclusion structural rather than a conditional someone can flip by accident. +# The comparison report and the raw BenchmarkDotNet artifacts are still published. +# +# Everything else - platform/VM provisioning, benchmark run model, noise controls - is identical to +# the other two pipelines; see eng/pipelines/perf/README.md. + +# Set the pipeline run name to the day-of-year and the daily run counter. +name: $(DayOfYear)$(Rev:rr) + +# Manual/queue-time only. A full perf run occupies a dedicated host for hours, and a switch +# experiment is an investigation someone opts into deliberately, never an automatic gate. +pr: none +trigger: none + +parameters: + + # The single runner-config switch under test. Baseline runs it false, current runs it true, so the + # reported delta is "what turning this switch ON does". Restricted to the switches the run scripts + # know how to stamp into the runner config; the scripts re-validate and fail fast on anything else. + - name: switchUnderTest + displayName: Switch under test (baseline=off vs current=on) + type: string + default: UseConnectionPoolV2 + values: + - UseConnectionPoolV2 + - UseOptimizedAsyncBehaviour + - UseManagedSniOnWindows + + # Target OS for the perf VM and the benchmark client. + - name: platform + displayName: Platform + type: string + default: linux + values: + - linux + - windows + + # The .NET runtime the benchmarks are executed against. Must be one of the target frameworks of + # the PerformanceTests project (net8.0/net9.0/net10.0). + - name: dotnetFramework + displayName: .NET Framework (TFM) + type: string + default: net9.0 + values: + - net8.0 + - net9.0 + - net10.0 + + # 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. + - name: testTimeoutMinutes + displayName: Test Timeout (minutes) + type: number + default: 180 + + # Percent difference (switch-on vs switch-off mean) the comparison flags. + - name: regressionThreshold + displayName: Difference Threshold (%) + type: number + default: 10 + + # Whether a confirmed "switch ON is slower than switch OFF" result should FAIL the run. + # + # NOTE: this maps onto the run scripts' --fail-on-regression gate, but it does NOT mean the same + # thing as it does in the other two pipelines. There, a regression means the branch got worse and + # failing is a quality gate. Here it is a RESULT: the switch made things slower. That is often + # exactly what you queued the run to find out, so this defaults to false and should only be enabled + # when you are asserting "this switch must not be a slowdown" (e.g. before flipping its default). + - name: failIfSwitchSlower + displayName: Fail run if the switch is slower + type: boolean + default: false + + # Benchmark run model (wiki 339 §2.2/§2.3/§2.6): + # interleaved -> run one benchmark unit at a time, switch-off and switch-on back-to-back, and + # confirm any flagged difference across N passes (best-of-N). Noise-resistant + # default, and especially valuable here because the two passes are otherwise + # identical, so any systematic drift between them is pure measurement noise. + # sequential -> legacy: run the whole switch-off suite, then the whole switch-on suite, compare. + - name: benchmarkRunMode + displayName: Benchmark run mode + type: string + default: interleaved + values: + - interleaved + - sequential + + # Best-of-N: total interleaved passes for a flagged unit before a difference is confirmed + # (1 disables confirmation). Only used when benchmarkRunMode = interleaved. + - name: confirmationRuns + displayName: Confirmation runs (best-of-N) + type: number + default: 3 + +# Fixed (non-configurable) constants for this pipeline. These are intentionally NOT parameters or +# library variables: they are invariant for the SqlClient perf pipelines. +# * buildConfiguration = Release - perf numbers are only meaningful in Release. +# * sourcesSubDir = dotnet-sqlclient - folder 'self' checks out into under the template's +# MULTI-REPO checkout ($(Build.SourcesDirectory)/); +# must match the ADO repository name. +# +# The other pipelines additionally expose UseManagedSniOnWindows / UseOptimizedAsyncBehaviour / +# UseConnectionPoolV2 as queue-time flags applied to BOTH passes. This pipeline deliberately does +# not: an experiment that varies one switch while others are also moved off their checked-in defaults +# produces a delta nobody can attribute. Every switch except the one under test is therefore left at +# its runnerconfig.jsonc default (which is what those parameters' defaults already are). +# +# NOTE: like sqlclient-perf-pr-pipeline.yml, this pipeline does NOT reference the 'ADX Cluster +# Variables' group and has no translate/ingest steps. See the header comment for why that is +# structural here rather than conditional. +variables: + + # Pre-computed testScriptArgs chunks. The Windows entry point is PowerShell and binds + # '-PascalCase' params, while the Linux one is bash and parses '--kebab-case' flags, so the + # argument STYLE differs by platform. Assembling the final args from these per-platform chunks + # keeps the single testScriptArgs below readable. No baseline-selector args appear here at all: + # --switch-under-test IS the baseline selector for this pipeline, and the run scripts reject it + # being combined with --baseline-version / --baseline-source-ref. + - ${{ if eq(parameters.platform, 'windows') }}: + - name: PerfArgsCommon + value: '-Configuration Release -Framework ${{ parameters.dotnetFramework }} -ResultsSubdir perf-results -RegressionThreshold ${{ parameters.regressionThreshold }} -RunMode ${{ parameters.benchmarkRunMode }} -ConfirmationRuns ${{ parameters.confirmationRuns }} -SwitchUnderTest ${{ parameters.switchUnderTest }}' + - ${{ else }}: + - name: PerfArgsCommon + value: '--configuration Release --framework ${{ parameters.dotnetFramework }} --results-subdir perf-results --regression-threshold ${{ parameters.regressionThreshold }} --run-mode ${{ parameters.benchmarkRunMode }} --confirmation-runs ${{ parameters.confirmationRuns }} --switch-under-test ${{ parameters.switchUnderTest }}' + + # Gate flag, only when the run is asserting the switch must not be a slowdown. + - ${{ if and(eq(parameters.platform, 'windows'), eq(parameters.failIfSwitchSlower, true)) }}: + - name: PerfArgsFail + value: '-FailOnRegression' + - ${{ elseif eq(parameters.failIfSwitchSlower, true) }}: + - name: PerfArgsFail + value: '--fail-on-regression' + - ${{ else }}: + - name: PerfArgsFail + value: '' + +# Reference the PerfTest repository that hosts the reusable extends template. Driver-team projects +# have been onboarded with the Agent Pools and Service Connections required to consume it. +resources: + repositories: + - repository: PerfTemplates + type: git + name: InternalDriverTools/PerfTest + ref: refs/heads/main + +# Consume the Perf Test Lab template. The template defines the whole job/stage structure; we only +# pass parameters into it. +extends: + template: v1/Perf.Test.Job.yml@PerfTemplates + parameters: + platform: ${{ parameters.platform }} + + # The entire driver source tree is copied to the VM so the benchmarks (which reference + # Microsoft.Data.SqlClient as a project) can be built from source against the current commit. + # Under the template's multi-repo checkout, 'self' lands in a repo-named subfolder. + testRootDir: $(Build.SourcesDirectory)/dotnet-sqlclient + + # Entry-point script (relative to testRootDir), selected by platform. Linux uses bash, Windows + # uses PowerShell, per the template's .sh/.ps1 convention. Same scripts as the other two perf + # pipelines: the switch experiment is just a different baseline selector. + ${{ if eq(parameters.platform, 'windows') }}: + testScript: eng/pipelines/perf/scripts/run-perf-tests.ps1 + ${{ else }}: + testScript: eng/pipelines/perf/scripts/run-perf-tests.sh + + # Arguments forwarded to the script. The script also reads SQL_SERVER, SQL_PASSWORD and + # PERF_CLIENT_CPUS directly from the VM session environment (injected by the template). Empty + # chunks collapse to harmless extra spaces that both PowerShell and bash arg parsing ignore. + testScriptArgs: '$(PerfArgsCommon) $(PerfArgsFail)' + + # Subfolder (relative to testRootDir on the VM) the script writes results into. The template + # copies this VM folder back to the agent, but always lands it at a fixed location: + # $(Build.ArtifactStagingDirectory)/results (and publishes it as the 'perf-results' artifact). + testResultsSubDir: perf-results + + testTimeoutMinutes: ${{ parameters.testTimeoutMinutes }} + jobName: SqlClientPerfExperiment + + # Post-test steps run on the agent after results have been copied back. The template already + # publishes the results artifact and attaches any top-level results/*.md as run summaries; this + # step additionally surfaces the BenchmarkDotNet markdown reports in the build log. There is no + # Kusto translation/ingestion here by design - see the header comment. + steps: + # Tag the build with the switch that was measured, so switch experiments are identifiable at a + # glance in the ADO build list and are never mistaken for an ordinary baseline comparison. + # Unlike the PR pipeline there is nothing to read back from the VM: the switch name is fixed at + # queue time, and both passes are the commit the run was queued on. + # NOTE: the tag intentionally has no ':' - build tags are placed in the request URL path and a + # colon trips ADO's "potentially dangerous Request.Path" filter. + - bash: | + echo "##vso[build.addbuildtag]Switch ${{ parameters.switchUnderTest }}" + displayName: 'Tag build with switch under test' + condition: succeededOrFailed() + + - task: Bash@3 + displayName: 'Show performance results' + condition: succeededOrFailed() + inputs: + targetType: filePath + filePath: $(Build.SourcesDirectory)/dotnet-sqlclient/eng/pipelines/perf/scripts/show_perf_results.sh + env: + RESULTS_DIR: $(Build.ArtifactStagingDirectory)/results diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs index a547ce0fd4..b2b69855b2 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs @@ -62,12 +62,6 @@ namespace Microsoft.Data.SqlClient.ConnectionPool internal sealed class ChannelDbConnectionPool : IDbConnectionPool, IDisposable { #region Fields - // Limits synchronous operations which depend on async operations on managed - // threads from blocking on all available threads, which would stop async tasks - // from being scheduled and cause deadlocks. Use ProcessorCount/2 as a balance - // between sync and async tasks. - private static SemaphoreSlim _syncOverAsyncSemaphore = new(Math.Max(1, Environment.ProcessorCount / 2)); - /// /// Tracks the number of instances of this class. Used to generate unique IDs for each instance. /// @@ -939,11 +933,23 @@ public bool TryGetConnection( if (taskCompletionSource is null) { // We're on the caller's thread, so the ambient transaction is directly observable. + Transaction? currentTransaction = ADP.GetCurrentTransaction(); + + // Fast path: when the pool can satisfy the request immediately, do it here rather + // than entering GetInternalConnection, which allocates a Task + // and a timer-backed CancellationTokenSource before it knows whether it will ever + // need to wait. See TryGetPooledConnectionInline. + connection = TryGetPooledConnectionInline(owningObject, currentTransaction); + if (connection is not null) + { + return true; + } + var task = GetInternalConnection( owningObject, async: false, timeout, - ADP.GetCurrentTransaction()); + currentTransaction); // When running synchronously, we are guaranteed that the task is already completed. // We don't need to guard the managed threadpool at this spot because we pass the async flag as false @@ -998,6 +1004,33 @@ public bool TryGetConnection( // processes pending opens on a dedicated non-thread-pool thread. Transaction? ambientTransaction = taskCompletionSource.Task.AsyncState as Transaction; + // Fast path: if the pool can satisfy this request right now, hand the connection back + // synchronously and report the request as completed. + // + // Returning true here (rather than completing the TaskCompletionSource and returning + // false) is what makes this cheap, and it is what + // WaitHandleDbConnectionPool.TryGetConnection does on its own inline hit. Reporting the + // open as incomplete sends SqlConnection.InternalOpenAsync down its asynchronous + // completion path, which allocates an OpenAsyncRetry, a CancellationTokenRegistration + // and a Tuple, and then schedules the continuation with + // ContinueWith(..., TaskScheduler.Default). That continuation costs a thread pool + // dispatch even when the TaskCompletionSource is already completed, so completing it + // inline would move the dispatch rather than remove it. Returning true instead lets + // InternalOpenAsync take its synchronous branch and skip all of it. + // + // The TaskCompletionSource is deliberately left untouched. The caller abandons it when + // the open completes synchronously, exactly as it does for the WaitHandle pool. + // + // Like v1's inline attempt (allowCreate: false), this never opens a physical + // connection, so the caller's thread is never blocked on network I/O. Exceptions + // propagate synchronously, which is also what the WaitHandle pool does from here. + DbConnectionInternal? pooled = TryGetPooledConnectionInline(owningObject, ambientTransaction); + if (pooled is not null) + { + connection = pooled; + return true; + } + Task.Run(async () => { if (taskCompletionSource.Task.IsCompleted) @@ -1431,6 +1464,78 @@ private void RemoveConnection(DbConnectionInternal connection) return null; } + /// + /// Attempts to satisfy a connection request from connections the pool already holds, + /// without blocking, waiting, or opening a physical connection. + /// + /// + /// + /// This is the fast path shared by the sync and async entry points of + /// . It performs the same two lookups the main loop in + /// begins with - the transacted store, then the idle + /// channel - and returns null the moment neither can satisfy the request, leaving the + /// caller to fall back to the full path. + /// + /// + /// Keeping this separate from is what makes it cheap. + /// That method is an async state machine that allocates a even + /// when it completes synchronously, and it creates a timer-backed + /// up front, before it knows whether it will ever + /// wait. Neither is needed to hand back a connection that is already sitting in the pool. + /// + /// + /// It deliberately does NOT call : that blocks on + /// network I/O, which must not happen on an async caller's thread. Callers that miss here + /// go through , which may open a connection. + /// + /// + /// The DbConnection that will own this internal connection. + /// The ambient transaction captured on the caller's thread, + /// or null when the caller is not inside a transaction. + /// An activated connection ready to be handed to the caller, or null when the pool + /// cannot satisfy the request without waiting or opening. + /// + /// Propagates any exception from activating or enlisting the connection. The connection is + /// returned to the pool before the exception escapes (see ). + /// + private DbConnectionInternal? TryGetPooledConnectionInline( + DbConnection owningConnection, + Transaction? ambientTransaction) + { + // When automatic enlistment is disabled the connection must never be bound to the + // ambient transaction, so we neither consult the transacted store nor hand the + // transaction to activation. Mirrors GetInternalConnection. + Transaction? transaction = HasTransactionAffinity ? ambientTransaction : null; + + DbConnectionInternal? connection = null; + + // A connection already enlisted in our transaction is always preferred, since reusing + // it avoids promoting the transaction to a distributed one. + if (transaction is not null) + { + connection = GetFromTransactedPool(transaction); + } + + // GetIdleConnection only returns connections that passed IsLiveConnection, and + // GetFromTransactedPool has already probed liveness, so no further validation is + // needed here. GetInternalConnection re-checks after its channel wait because that + // wait can hand back a connection that bypassed both filters. + connection ??= GetIdleConnection(); + + if (connection is null) + { + return null; + } + + // Counted before activation for the same reason as GetInternalConnection: if + // PrepareConnection fails it returns the connection to the pool, which emits the + // matching soft disconnect. Counting after would leave that disconnect unpaired and + // drive the active-soft-connects gauge negative. + Metrics.SoftConnectRequest(); + PrepareConnection(owningConnection, connection, transaction); + return connection; + } + /// /// Gets an internal connection from the pool, either by retrieving an idle connection or opening a new one. /// @@ -1681,30 +1786,30 @@ private void SweepEmancipatedConnections() /// The connection read from the channel. private DbConnectionInternal? ReadChannelSyncOverAsync(CancellationToken cancellationToken) { - // If there are no connections in the channel, then ReadAsync will block until one is available. - // Channels doesn't offer a sync API, so running ReadAsync synchronously on this thread may spawn - // additional new async work items in the managed thread pool if there are no items available in the - // channel. We need to ensure that we don't block all available managed threads with these child - // tasks or we could deadlock. Prefer to block the current user-owned thread, and limit throughput - // to the managed threadpool. - - _syncOverAsyncSemaphore.Wait(cancellationToken); - try - { - ConfiguredValueTaskAwaitable.ConfiguredValueTaskAwaiter awaiter = - _idleChannel.ReadAsync(cancellationToken).ConfigureAwait(false).GetAwaiter(); - using ManualResetEventSlim mres = new ManualResetEventSlim(false, 0); - - // Cancellation happens through the ReadAsync call, which will complete the task. - // Even a failed task will complete and set the ManualResetEventSlim. - awaiter.UnsafeOnCompleted(() => mres.Set()); - mres.Wait(CancellationToken.None); - return awaiter.GetResult(); - } - finally - { - _syncOverAsyncSemaphore.Release(); - } + // Channel exposes no synchronous blocking read, so a sync caller has to block on the + // async read. ValueTask cannot be blocked on directly (GetResult throws if the operation + // has not completed), so materialize a Task and block on that. + // + // Blocking through the Task is deliberate rather than incidental. The idle channel is + // created without AllowSynchronousContinuations, so the continuation that hands this + // thread its connection is queued to the thread pool instead of running inline on the + // returning thread. Task-based blocking notifies the thread pool that this thread is + // stalled, which lets it inject a worker to run that continuation promptly. Blocking on + // an opaque primitive (ManualResetEventSlim, SemaphoreSlim) hides the stall: when every + // pool thread is parked here, nothing runs the continuations until the thread pool's + // slow injection heuristic fires on its own, adding hundreds of milliseconds to each + // acquire while the pool is saturated. + // + // For the same reason there is no semaphore limiting how many threads may be in here at + // once. Such a gate cannot prevent sync work from consuming thread pool threads, because + // waiting on it blocks the calling thread just as opaquely; it only moves the invisible + // block one frame up and starves the very continuations these waiters depend on. + // + // Blocking on a Task is the classic sync-over-async deadlock shape when a single-threaded + // SynchronizationContext is installed (WPF, WinForms, legacy ASP.NET). It is safe here + // only because IdleConnectionChannel.ReadAsync awaits with ConfigureAwait(false), so its + // completion never needs the context this thread is holding. Do not remove that. + return _idleChannel.ReadAsync(cancellationToken).AsTask().GetAwaiter().GetResult(); } /// diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/IdleConnectionChannel.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/IdleConnectionChannel.cs index 801c0e7c2f..df4c1cbc95 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/IdleConnectionChannel.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/IdleConnectionChannel.cs @@ -95,6 +95,13 @@ internal bool TryRead(out DbConnectionInternal? connection) /// Asynchronously reads a value from the channel. /// Decrements the idle count when a non-null connection is read. /// + /// + /// The ConfigureAwait(false) below is load bearing. Sync callers reach this method + /// through ChannelDbConnectionPool.ReadChannelSyncOverAsync, which blocks on the + /// returned operation. If this resumption captured a single-threaded + /// (WPF, WinForms, legacy ASP.NET), completing it would + /// require the very thread that is blocked waiting for it, deadlocking the caller. + /// internal async ValueTask ReadAsync(CancellationToken cancellationToken) { var connection = await _reader.ReadAsync(cancellationToken).ConfigureAwait(false); diff --git a/src/Microsoft.Data.SqlClient/tests/PerformanceTests/BenchmarkRunners/ConnectionPoolChurnRunner.cs b/src/Microsoft.Data.SqlClient/tests/PerformanceTests/BenchmarkRunners/ConnectionPoolChurnRunner.cs index ca1930e451..d2b94cea14 100644 --- a/src/Microsoft.Data.SqlClient/tests/PerformanceTests/BenchmarkRunners/ConnectionPoolChurnRunner.cs +++ b/src/Microsoft.Data.SqlClient/tests/PerformanceTests/BenchmarkRunners/ConnectionPoolChurnRunner.cs @@ -20,6 +20,22 @@ namespace Microsoft.Data.SqlClient.PerformanceTests /// concern for the new ChannelDbConnectionPool, which aims to avoid extra allocations /// on the hot path (issue #3356). /// + /// This overlaps in shape — + /// the inner loop is identical — but not in purpose, and the two are not + /// interchangeable. Being single-threaded, this runner has no scheduling or wake-up + /// component, which makes it far more sensitive: its sync and async variants have + /// agreed to within 0.3 percentage points, whereas the concurrent runner's own + /// duplicate-workload parameter pairs have disagreed by around 20. Use this one to + /// decide whether per-checkout cost or allocation moved, and the concurrent runner to + /// decide whether hand-off between threads moved. A regression there with a flat result + /// here points at scheduling rather than at the checkout path. + /// + /// Adding a Parallelism=1 case to that runner would not replace this one. Its + /// [Params] are class-wide, so a single-threaded row would also be generated for + /// benchmarks it makes meaningless (MixedSyncAsyncContention would have no sync + /// worker left to mix in), and this runner's higher operation count and separate + /// iteration settings are what keep the signal clean. + /// /// The pool implementation (legacy vs V2) is a process-level choice — see the remarks /// on . Run twice (UseConnectionPoolV2 false /// then true) to compare. @@ -36,6 +52,27 @@ public class ConnectionPoolChurnRunner : BaseRunner [Params(1000)] public int OpsPerInvocation { get; set; } + /// + /// How many idle connections the pool holds while the single caller churns against it. + /// + /// + /// This is not a redundant axis, because the two pools order idle connections + /// differently: the legacy pool pops from a ConcurrentStack (LIFO) while the V2 + /// pool reads from an unbounded Channel (FIFO). At depth 1 that difference is + /// invisible, since both hand back the only connection there is. At a realistic depth + /// the legacy pool keeps returning the connection just released — one hot object — while + /// the V2 pool cycles through every idle connection in turn, touching all of their + /// buffers and parser state. Depth is therefore the axis that exposes reuse locality, + /// and measuring only depth 1 would hide it entirely. + /// + /// The middle depth is what makes the axis diagnostic rather than merely directional. + /// Depths 1 and 100 alone show only that reuse locality degrades somewhere in between; + /// they cannot distinguish a threshold (cost appears once the pool exceeds some working + /// set) from a gradient (cost grows smoothly with depth). Those imply different fixes. + /// + [Params(1, 10, 100)] + public int PoolDepth { get; set; } + private string _connectionString; [GlobalSetup] @@ -51,16 +88,48 @@ public void Setup() { Pooling = true, MaxPoolSize = 100, - // Pre-warm a single connection so the very first checkout is served from - // the pool rather than establishing a physical connection. - MinPoolSize = 1 + // Pin the floor at the requested depth so pruning cannot shrink the pool back + // down mid-run and change what the benchmark is measuring. + MinPoolSize = PoolDepth, + // Matches ConnectionPoolStressRunner. At the larger PoolDepth, setup establishes + // a hundred physical connections back to back, and the default 15s is tight for + // that against a loaded remote server. + ConnectTimeout = 60 }; _connectionString = builder.ConnectionString; - // Force the pool to exist and hold at least one idle connection. - using var warm = new SqlConnection(_connectionString); - warm.Open(); - warm.Close(); + PrewarmPool(); + } + + /// + /// Fills the pool with idle connections before measurement, so + /// every measured open is a pure checkout. + /// + /// + /// All connections are opened before any is released. Releasing as we go would let the + /// pool hand the same idle connection straight back and create only one, which would + /// silently collapse every depth to 1. MinPoolSize alone is not enough either: + /// both pools backfill it on a background task, so the first measured iteration would + /// otherwise still be racing that warm-up. + /// + private void PrewarmPool() + { + var warm = new SqlConnection[PoolDepth]; + try + { + for (int i = 0; i < warm.Length; i++) + { + warm[i] = new SqlConnection(_connectionString); + warm[i].Open(); + } + } + finally + { + foreach (var conn in warm) + { + conn?.Dispose(); + } + } } [GlobalCleanup] diff --git a/src/Microsoft.Data.SqlClient/tests/PerformanceTests/BenchmarkRunners/ConnectionPoolContentionRunner.cs b/src/Microsoft.Data.SqlClient/tests/PerformanceTests/BenchmarkRunners/ConnectionPoolContentionRunner.cs index 899d25f64c..c13bdb4d3d 100644 --- a/src/Microsoft.Data.SqlClient/tests/PerformanceTests/BenchmarkRunners/ConnectionPoolContentionRunner.cs +++ b/src/Microsoft.Data.SqlClient/tests/PerformanceTests/BenchmarkRunners/ConnectionPoolContentionRunner.cs @@ -30,6 +30,14 @@ namespace Microsoft.Data.SqlClient.PerformanceTests /// remarks on . Run twice (UseConnectionPoolV2 /// false then true) to compare. /// + /// Each sync workload is measured twice, once on threadpool threads + /// () and once on dedicated threads + /// (). Threadpool threads are the + /// realistic case, since sync database calls in ASP.NET run on them, and they are the + /// only configuration that can expose a waiter wake path which depends on the + /// threadpool having a free thread. Dedicated threads isolate the pool's own cost. The + /// pair separates a pool regression from a scheduling one. + /// /// Related issues: #601, #979, #3356 /// public class ConnectionPoolContentionRunner : BaseRunner @@ -41,13 +49,24 @@ public class ConnectionPoolContentionRunner : BaseRunner public int Parallelism { get; set; } /// - /// Maximum pool size, exercised across three regimes relative to - /// : larger than the worker count (idle spare - /// connections, no contention), equal to it (fully subscribed, no contention), and - /// smaller than it (pool exhaustion forces workers to wait for a connection to be - /// returned — back-pressure). + /// Maximum pool size, exercised across the full demand/capacity ladder relative to + /// : larger than the worker count (idle spare connections, no + /// contention), equal to it (fully subscribed, no contention), and progressively smaller + /// than it (pool exhaustion forces workers to wait for a connection to be returned — + /// back-pressure). /// - [Params(100, 50, 10)] + /// + /// At 50 these give demand/capacity ratios of 0.25, 0.5, 1, 2 + /// and 5. The intermediate over-subscribed step (ratio 2) is deliberate: back-pressure + /// regressions show up only once demand exceeds capacity, and without a point between + /// "fully subscribed" and "five times over" there is no way to tell how quickly the cost + /// grows once the pool starts running dry. + /// + /// 200 is included because it is the most commonly configured explicit Max Pool Size in + /// production, so the least-contended row corresponds to a real deployment rather than + /// only to the driver default. + /// + [Params(200, 100, 50, 25, 10)] public int MaxPoolSize { get; set; } /// @@ -105,7 +124,7 @@ public Task SteadyStateOpenQueryClose() conn.Open(); using var cmd = conn.CreateCommand(); cmd.CommandText = "SELECT 1"; - _ = cmd.ExecuteScalar(); + var result = cmd.ExecuteScalar(); // Dispose returns the connection to the pool. } }); @@ -114,6 +133,45 @@ public Task SteadyStateOpenQueryClose() return Task.WhenAll(tasks); } + /// + /// Same workload as , but driven by dedicated + /// threads instead of threadpool threads. + /// + /// Read the two together. A sync Open() that has to wait blocks whichever + /// thread it is running on. On threadpool threads that competes with the threadpool + /// itself, because a pool implementation whose waiter wake-up depends on a queued + /// continuation cannot make progress while every thread is blocked in a wait: the + /// wake-up is stuck behind thread injection, which adds roughly a second per stall. + /// Dedicated threads remove that coupling, so this variant measures the pool's + /// intrinsic checkout/return cost with the scheduler taken out of the picture. + /// + /// A regression in both points at the pool itself. A regression only in the + /// threadpool variant points at the waiter wake path and shows up as tail latency + /// rather than a shifted median, so compare the distribution and not just the mean. + /// + [Benchmark] + public Task SteadyStateOpenQueryCloseDedicatedThreads() + { + var tasks = new Task[Parallelism]; + for (int i = 0; i < Parallelism; i++) + { + tasks[i] = Task.Factory.StartNew(() => + { + for (int op = 0; op < OpsPerWorker; op++) + { + using var conn = new SqlConnection(_connectionString); + conn.Open(); + using var cmd = conn.CreateCommand(); + cmd.CommandText = "SELECT 1"; + _ = cmd.ExecuteScalar(); + // Dispose returns the connection to the pool. + } + }, TaskCreationOptions.LongRunning); + } + + return Task.WhenAll(tasks); + } + [Benchmark] public Task SteadyStateOpenQueryCloseAsync() { diff --git a/src/Microsoft.Data.SqlClient/tests/PerformanceTests/BenchmarkRunners/ConnectionPoolRampRunner.cs b/src/Microsoft.Data.SqlClient/tests/PerformanceTests/BenchmarkRunners/ConnectionPoolRampRunner.cs new file mode 100644 index 0000000000..344884fbc3 --- /dev/null +++ b/src/Microsoft.Data.SqlClient/tests/PerformanceTests/BenchmarkRunners/ConnectionPoolRampRunner.cs @@ -0,0 +1,177 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.Threading; +using System.Threading.Tasks; +using BenchmarkDotNet.Attributes; + +namespace Microsoft.Data.SqlClient.PerformanceTests +{ + /// + /// Measures how quickly a cold pool can ramp up to N physical connections when N + /// callers arrive simultaneously and all of them need a connection at the same time. + /// + /// This is the workload the ChannelDbConnectionPool (V2) was designed for. The legacy + /// WaitHandleDbConnectionPool guards creation with a Semaphore(1, 1), so a cold + /// burst of N callers establishes physical connections one at a time: total latency is + /// roughly N x connect latency. V2 has no such gate, so the opens overlap and total + /// latency approaches a single connect. + /// + /// Contrast with , which also + /// starts from a cold pool but releases each connection immediately. Because nothing is + /// held, one physical connection can satisfy every caller in turn, so that benchmark + /// rewards a pool that grows as slowly as possible and penalizes concurrent creation. + /// Holding each connection until every caller has one removes that artifact: the pool + /// genuinely needs N connections, and the only variable left is how fast it can open + /// them. + /// + /// is always larger than so no + /// caller ever waits for a connection to be returned. Back-pressure on a saturated pool + /// is covered separately by . + /// + /// The pool implementation (legacy vs V2) is a process-level choice - see the remarks on + /// . Run twice (UseConnectionPoolV2 false then + /// true) to compare. + /// + /// Related issue: #3356 + /// + public class ConnectionPoolRampRunner : BaseRunner + { + /// + /// Number of callers that arrive simultaneously against a cold pool. Each one holds + /// its connection until all of them have connected, so the pool must open exactly + /// this many physical connections. + /// + [Params(10, 25, 50, 100)] + public int Parallelism { get; set; } + + /// + /// Max pool size. Deliberately larger than every value so + /// the ramp is never bounded by pool capacity. + /// + [Params(200)] + public int MaxPoolSize { get; set; } + + private string _connectionString; + + /// + /// Upper bound on the rendezvous wait. The rendezvous is released on the failure path + /// too, so this should never be reached; it exists so that a bug in the barrier fails + /// the run quickly instead of hanging the perf pipeline indefinitely. + /// + private static readonly TimeSpan s_rampTimeout = TimeSpan.FromMinutes(2); + + [GlobalSetup] + public void Setup() + { + Console.WriteLine( + "[ConnectionPoolRampRunner] Pool implementation: " + + (s_config.UseConnectionPoolV2 + ? "ChannelDbConnectionPool (V2)" + : "WaitHandleDbConnectionPool (legacy)")); + + var builder = new SqlConnectionStringBuilder(s_config.ConnectionString) + { + Pooling = true, + MaxPoolSize = MaxPoolSize, + // No pre-warming: every iteration must establish its own connections. + MinPoolSize = 0, + ConnectTimeout = 60 + }; + _connectionString = builder.ConnectionString; + } + + [IterationSetup] + public void IterationSetup() + { + // Start every iteration from a cold pool so the measurement is the ramp itself. + SqlConnection.ClearAllPools(); + } + + [GlobalCleanup] + public void Cleanup() => SqlConnection.ClearAllPools(); + + /// + /// Async cold-start ramp. All callers open concurrently and hold until the last one + /// has connected. + /// + [Benchmark] + public async Task ColdStartRampAsync() + { + using var allConnected = new CountdownEvent(Parallelism); + var release = new TaskCompletionSource( + TaskCreationOptions.RunContinuationsAsynchronously); + + var tasks = new Task[Parallelism]; + for (int i = 0; i < Parallelism; i++) + { + tasks[i] = Task.Run(async () => + { + using var conn = new SqlConnection(_connectionString); + try + { + await conn.OpenAsync(); + } + finally + { + // Signal from a finally so a caller that fails to open still counts as + // arrived. Otherwise the countdown never reaches zero, the release is + // never set, and every other caller awaits forever. + if (allConnected.Signal()) + { + release.TrySetResult(true); + } + } + + // Hold the connection until every caller has one, forcing the pool to + // grow to Parallelism physical connections. + await release.Task.WaitAsync(s_rampTimeout); + // Dispose returns the connection to the pool. + }); + } + + await Task.WhenAll(tasks); + } + + /// + /// Sync cold-start ramp. Uses dedicated threads rather than thread pool threads so + /// the measurement reflects pool ramp latency rather than thread pool injection + /// delay, which would otherwise dominate once the callers block. + /// + [Benchmark] + public void ColdStartRamp() + { + using var allConnected = new CountdownEvent(Parallelism); + + var tasks = new Task[Parallelism]; + for (int i = 0; i < Parallelism; i++) + { + tasks[i] = Task.Factory.StartNew(() => + { + using var conn = new SqlConnection(_connectionString); + try + { + conn.Open(); + } + finally + { + // See ColdStartRampAsync: signalling from a finally keeps a failed + // open from stranding every other caller in Wait(). + allConnected.Signal(); + } + + if (!allConnected.Wait(s_rampTimeout)) + { + throw new TimeoutException( + $"Cold-start ramp did not reach {Parallelism} connections within {s_rampTimeout}."); + } + // Dispose returns the connection to the pool. + }, TaskCreationOptions.LongRunning); + } + + Task.WaitAll(tasks); + } + } +} diff --git a/src/Microsoft.Data.SqlClient/tests/PerformanceTests/BenchmarkRunners/ConnectionPoolStressRunner.cs b/src/Microsoft.Data.SqlClient/tests/PerformanceTests/BenchmarkRunners/ConnectionPoolStressRunner.cs index 9dad6f41d0..4c4c7e63d9 100644 --- a/src/Microsoft.Data.SqlClient/tests/PerformanceTests/BenchmarkRunners/ConnectionPoolStressRunner.cs +++ b/src/Microsoft.Data.SqlClient/tests/PerformanceTests/BenchmarkRunners/ConnectionPoolStressRunner.cs @@ -10,12 +10,20 @@ namespace Microsoft.Data.SqlClient.PerformanceTests { /// /// Stress-tests the connection pool with randomized parallel access patterns: - /// - Massive concurrent open/close churn + /// - Massive concurrent open/close churn, both sync and async /// - Randomized hold durations simulating real workloads /// - Mixed sync/async callers competing for pooled connections /// - Connection reuse with interleaved queries /// - Pool exhaustion and recovery under pressure /// + /// The pool is pre-warmed to full capacity in and is deliberately + /// not cleared between iterations, so these benchmarks measure steady-state + /// checkout and return rather than physical connection establishment. Establishing + /// connections costs orders of magnitude more than a pooled checkout (milliseconds versus + /// microseconds), so leaving any creation in the measured body swamps the pool cost this + /// class exists to measure. Cold-start behaviour is covered separately and deliberately by + /// . + /// /// Related issues: #601, #979, #3356 /// public class ConnectionPoolStressRunner : BaseRunner @@ -26,21 +34,41 @@ public class ConnectionPoolStressRunner : BaseRunner /// /// Number of concurrent tasks hammering the pool. /// - [Params(10, 20, 25)] + /// + /// Two values spanning 5x rather than a tighter cluster. Every benchmark here runs at + /// every , so this axis is multiplied across seven benchmarks + /// and intermediate values buy far less than they cost — adjacent values in a narrow + /// range mostly re-measure the same regime. + /// + [Params(10, 50)] public int Parallelism { get; set; } /// /// Max pool size — controls how many physical connections the pool can hold. - /// When Parallelism exceeds this, tasks must wait for a free connection. /// + /// + /// The pool is pre-warmed to this many connections and pinned there by an equal + /// Min Pool Size, so this is the number of connections actually resident for the + /// whole run rather than just a ceiling. The limit itself is only reached by + /// , which deliberately oversubscribes it, and by the + /// fully-subscribed corner where equals this value. Everywhere + /// else the pool never saturates, so this parameter mostly varies how many idle + /// connections the checkout path is choosing among; treat a spread between two + /// MaxPoolSize values at the same Parallelism as a noise estimate rather than a real + /// effect. The fully-subscribed corner is the exception — there the spread is a real + /// effect, because one side has idle spares to choose from and the other has none. + /// [Params(50, 100)] public int MaxPoolSize { get; set; } [GlobalSetup] public void Setup() { + // Pin Min Pool Size to Max Pool Size so the pool holds full capacity for the whole + // run: pruning cannot shrink it back down, and no benchmark body has to establish a + // physical connection. _connectionString = s_config.ConnectionString + - $";Pooling=True;Max Pool Size={MaxPoolSize};Min Pool Size=5;Connect Timeout=60"; + $";Pooling=True;Max Pool Size={MaxPoolSize};Min Pool Size={MaxPoolSize};Connect Timeout=60"; // Create a small table for query workloads. // Hash the machine name instead of using it verbatim: hostnames can be long enough @@ -48,22 +76,55 @@ public void Setup() // than using Math.Abs, which throws OverflowException when the hash is int.MinValue. string machineHash = ((uint)Environment.MachineName.GetHashCode()).ToString("x8"); _tableName = $"[perf_PoolStress_{machineHash}_{Guid.NewGuid():N}]"; - using var conn = new SqlConnection(_connectionString); - conn.Open(); - using var cmd = new SqlCommand( - $"CREATE TABLE {_tableName} (Id INT IDENTITY PRIMARY KEY, Val INT)", conn); - cmd.ExecuteNonQuery(); - // Seed a few rows so SELECT queries return data - using var insert = new SqlCommand( - $"INSERT INTO {_tableName} (Val) VALUES (1),(2),(3),(4),(5)", conn); - insert.ExecuteNonQuery(); + // Scoped so this connection is back in the pool before PrewarmPool runs: that method + // holds MaxPoolSize connections at once, so an extra live one would hit the cap and + // block until Connect Timeout. + { + using var conn = new SqlConnection(_connectionString); + conn.Open(); + using var cmd = new SqlCommand( + $"CREATE TABLE {_tableName} (Id INT IDENTITY PRIMARY KEY, Val INT)", conn); + cmd.ExecuteNonQuery(); + + // Seed a few rows so SELECT queries return data + using var insert = new SqlCommand( + $"INSERT INTO {_tableName} (Val) VALUES (1),(2),(3),(4),(5)", conn); + insert.ExecuteNonQuery(); + } + + PrewarmPool(); } - [IterationCleanup] - public void IterationCleanup() + /// + /// Fills the pool to live connections before any measurement + /// starts, so benchmark bodies only ever exercise checkout and return. + /// + /// + /// Every connection is opened before any is released. Releasing as we go would let the + /// pool hand the same idle connection back repeatedly and create only one, which is the + /// whole failure this is meant to avoid. Min Pool Size alone is not enough either: + /// it is backfilled lazily, so the first measured iteration would still pay for creation. + /// + private void PrewarmPool() { - SqlConnection.ClearAllPools(); + var warm = new SqlConnection[MaxPoolSize]; + try + { + for (int i = 0; i < warm.Length; i++) + { + warm[i] = new SqlConnection(_connectionString); + warm[i].Open(); + } + } + finally + { + // Returns them all to the pool; Min Pool Size keeps them resident from here on. + foreach (var conn in warm) + { + conn?.Dispose(); + } + } } [GlobalCleanup] @@ -79,13 +140,34 @@ public void Cleanup() /// /// Pure open/close churn — every task opens a pooled connection, immediately closes it, - /// and repeats. Measures raw pool checkout/return throughput under contention. - /// The per-task loop count scales with MaxPoolSize so total checkouts stay proportional - /// to pool capacity regardless of how the [Params] values change. + /// and repeats. With the pool pre-warmed to full capacity, every open is a pure checkout, + /// so this measures the pool's acquire/return path under concurrency. /// + /// + /// + /// Because nothing happens between checkout and return, the tasks stay phase-locked and + /// the idle channel oscillates around empty even though the pool is full. That makes this + /// benchmark unusually sensitive to how a returned connection is handed to a waiting + /// caller: under the V2 pool an inline TryRead miss parks the caller in + /// ReadAsync, so it resumes on a threadpool continuation. Read it as a + /// zero-hold-time worst case for wake-up scheduling, not as typical application + /// behaviour: real callers do some work while holding a connection, which decorrelates + /// returns from checkouts and lets the fast path hit. + /// + /// + /// Compare against for the same loop with no + /// concurrency, and for concurrency with a + /// realistic hold time. A regression here alongside flat or improved results in those two + /// indicates a change in wake-up scheduling rather than in checkout cost. + /// + /// [Benchmark] - public async Task RapidFireOpenClose() + public async Task RapidFireOpenCloseAsync() { + // NOTE: the Math.Max floor dominates for most [Params] combinations, so total + // checkouts do NOT scale cleanly with pool capacity — at Parallelism 20 and 25 both + // MaxPoolSize values run an identical workload, which makes the spread between them + // a useful read on this benchmark's own noise floor. int iterationsPerTask = Math.Max(20, MaxPoolSize / Math.Max(1, Parallelism) * 4); var tasks = new Task[Parallelism]; for (int i = 0; i < Parallelism; i++) @@ -104,7 +186,40 @@ public async Task RapidFireOpenClose() } /// - /// Randomized hold — each task opens a connection, holds it for a random duration + /// Sync counterpart to : the same zero-hold churn, but + /// every checkout goes through the blocking Open() path. + /// + /// + /// Worth measuring separately because the two paths diverge inside the pool. The V2 pool + /// has no synchronous channel read, so a sync caller that misses the inline fast path has + /// to run the async wait synchronously, which is a materially different cost from + /// awaiting it. With the pool pre-warmed and never saturated by these + /// values, this stays on the fast path and so measures + /// concurrent sync checkout without the wake-up behaviour that + /// covers under saturation. + /// + /// Workers run on threadpool threads deliberately: sync database calls in ASP.NET run + /// there, so that is the configuration whose behaviour actually matters. + /// + [Benchmark] + public async Task RapidFireOpenCloseSync() + { + int iterationsPerTask = Math.Max(20, MaxPoolSize / Math.Max(1, Parallelism) * 4); + var tasks = new Task[Parallelism]; + for (int i = 0; i < Parallelism; i++) + { + tasks[i] = Task.Run(() => + { + for (int j = 0; j < iterationsPerTask; j++) + { + using var conn = new SqlConnection(_connectionString); + conn.Open(); + // immediate return to pool + } + }); + } + await Task.WhenAll(tasks); + } /// (0-50ms), optionally runs a query, then returns it. Simulates realistic mixed /// workloads where some connections are held briefly and others longer. /// @@ -214,11 +329,18 @@ public async Task MultiCommandReuse() /// wait. Measures how well the pool handles back-pressure when all connections are /// checked out and callers are queued. /// + /// + /// sets how far the pool is oversubscribed, and so how deep + /// the queue of waiting callers gets. It is the queue depth rather than the concurrency + /// level here, because saturating the pool already takes + /// tasks before any caller has to wait at all. + /// [Benchmark] public async Task PoolExhaustionRecovery() { - // Ensure we exceed pool capacity - int taskCount = Math.Max(Parallelism, MaxPoolSize * 2); + // Saturate the pool, then oversubscribe it by Parallelism so exactly that many + // callers are queued waiting for a connection to come back. + int taskCount = MaxPoolSize + Parallelism; var tasks = new Task[taskCount]; for (int i = 0; i < taskCount; i++) { diff --git a/src/Microsoft.Data.SqlClient/tests/PerformanceTests/BenchmarkRunners/ConnectionPoolThreadPoolPressureRunner.cs b/src/Microsoft.Data.SqlClient/tests/PerformanceTests/BenchmarkRunners/ConnectionPoolThreadPoolPressureRunner.cs new file mode 100644 index 0000000000..187fb72d86 --- /dev/null +++ b/src/Microsoft.Data.SqlClient/tests/PerformanceTests/BenchmarkRunners/ConnectionPoolThreadPoolPressureRunner.cs @@ -0,0 +1,249 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.Collections.Generic; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using BenchmarkDotNet.Attributes; + +namespace Microsoft.Data.SqlClient.PerformanceTests +{ + /// + /// Measures a saturated pool driven by sync callers on threadpool threads, with the + /// threadpool's minimum worker count pinned so the result is reproducible. + /// + /// A sync Open() against a saturated pool blocks its thread. When those threads + /// are threadpool threads, a pool whose waiter wake-up requires a queued continuation + /// cannot make progress: every thread is blocked in a wait, so the wake-up sits in the + /// queue until the threadpool injects another thread. + /// + /// Injection is not a single mechanism. Since .NET 6 the runtime recognises cooperative + /// blocking and compensates on a fast path — up to one thread per processor immediately + /// with no delay, then in 25ms steps capped at 250ms. Only once that budget is spent does + /// the caller fall back to the gate thread's starvation detection, which adds one thread + /// per 500ms cycle. Stalls here are therefore tens to a few hundred milliseconds, not the + /// whole seconds the pre-.NET 6 gate-thread-only path would have cost. + /// + /// covers the same shape at the default + /// threadpool floor, which makes it dependent on hill-climbing timing and therefore + /// noisy. Pinning the floor turns that into a controlled comparison: + /// + /// - below guarantees the + /// threadpool starts starved, so the wake path is exercised on every run. + /// - above pre-creates enough + /// threads that injection never gates progress. This is the control: a pool that only + /// regresses in the starved configuration has a wake-path problem, not a throughput + /// problem. + /// + /// The effect is tail latency, not a shifted median, so compare distributions rather + /// than means alone. + /// + /// A regression here is largely a statement about application configuration rather than a + /// pool defect, but the starved configuration is not exotic. The threadpool's default + /// minimum is , and that honours cgroup CPU quotas, + /// so a service in a 1-2 vCPU container runs with a floor of 1 or 2 by default and ASP.NET + /// Core never raises it. Blocking more threads than the floor is the common case for sync + /// callers, not a misconfiguration they opted into. This runner exists to characterise + /// where that boundary is and to catch it moving, not to drive the delta to zero. + /// + /// The pool implementation (legacy vs V2) is a process-level choice - see the remarks on + /// . Run twice (UseConnectionPoolV2 false then + /// true) to compare. + /// + /// Related issue: #3356 + /// + public class ConnectionPoolThreadPoolPressureRunner : BaseRunner + { + /// + /// Number of concurrent sync workers, all running on threadpool threads. + /// + [Params(50)] + public int Parallelism { get; set; } + + /// + /// Max pool size. Deliberately smaller than so most + /// workers must block waiting for a connection to be returned. Without that + /// back-pressure nobody waits and the wake path is never exercised. + /// + [Params(10)] + public int MaxPoolSize { get; set; } + + /// + /// Threadpool minimum worker thread count, pinned for the duration of the run. + /// + /// + /// Sourced from rather than fixed constants, + /// because this number is only meaningful relative to + /// — see that property for why. + /// + [ParamsSource(nameof(MinWorkerThreadsValues))] + public int MinWorkerThreads { get; set; } + + /// + /// The minimum worker counts to sweep, expressed as multiples of + /// . + /// + /// + /// Fixed constants would not survive a change of machine. The processor count is both + /// the value this parameter displaces (it is the runtime's own default floor) and the + /// size of the runtime's immediate cooperative-blocking injection budget, so the same + /// absolute number means "starved" on one host and "generous" on another. A constant + /// chosen to starve a 16-core benchmark machine would quietly stop starving anything on + /// a 4-core developer box, and the benchmark would keep reporting numbers that no longer + /// measure the wake path. + /// + /// The multiples map onto real deployments: a quarter of the processor count stands in + /// for the default floor of a 2-4 vCPU container, 1x is the runtime default that almost + /// every application actually runs, 2x is the most common explicit multiplier in shipped + /// code, and 8x is the control — comfortably above so + /// injection never gates progress. + /// + /// Floored at 1 because SetMinThreads rejects 0, and de-duplicated because the + /// lower multiples collapse together on very small hosts. + /// + public static IEnumerable MinWorkerThreadsValues => + new[] + { + Environment.ProcessorCount / 4, + Environment.ProcessorCount, + Environment.ProcessorCount * 2, + Environment.ProcessorCount * 8, + } + .Select(static value => Math.Max(1, value)) + .Distinct(); + + /// + /// Number of open/query/close operations each worker performs per invocation. + /// + [Params(20)] + public int OpsPerWorker { get; set; } + + private string _connectionString; + private int _originalMinWorkerThreads; + private int _originalMinCompletionPortThreads; + + [GlobalSetup] + public void Setup() + { + Console.WriteLine( + "[ConnectionPoolThreadPoolPressureRunner] Pool implementation: " + + (s_config.UseConnectionPoolV2 + ? "ChannelDbConnectionPool (V2)" + : "WaitHandleDbConnectionPool (legacy)")); + + ThreadPool.GetMinThreads( + out _originalMinWorkerThreads, out _originalMinCompletionPortThreads); + SetMinWorkerThreads(MinWorkerThreads, _originalMinCompletionPortThreads); + + var builder = new SqlConnectionStringBuilder(s_config.ConnectionString) + { + Pooling = true, + MaxPoolSize = MaxPoolSize, + MinPoolSize = 0 + }; + _connectionString = builder.ConnectionString; + } + + [GlobalCleanup] + public void Cleanup() + { + // Restoring matters beyond this benchmark: the floor is process-wide, so leaving it + // raised would silently change the conditions for every runner that follows in the + // same process. + SetMinWorkerThreads( + _originalMinWorkerThreads, _originalMinCompletionPortThreads); + } + + /// + /// Pins the threadpool worker floor, failing loudly if it does not take effect. + /// + /// is the only variable this benchmark manipulates, so a + /// refused or clamped request would leave every configuration running at the same floor + /// and produce a comparison that looks valid but measures nothing. The return value alone + /// is not sufficient evidence, so the value is also read back. + /// + private static void SetMinWorkerThreads(int workerThreads, int completionPortThreads) + { + if (!ThreadPool.SetMinThreads(workerThreads, completionPortThreads)) + { + throw new InvalidOperationException( + $"ThreadPool.SetMinThreads({workerThreads}, {completionPortThreads}) was refused. " + + "The threadpool floor is this benchmark's independent variable, so the run would " + + "otherwise report a comparison that did not actually vary it."); + } + + ThreadPool.GetMinThreads(out int actualWorkerThreads, out _); + if (actualWorkerThreads != workerThreads) + { + throw new InvalidOperationException( + $"ThreadPool.SetMinThreads({workerThreads}, ...) reported success but the floor " + + $"read back as {actualWorkerThreads}. The runtime clamped the request, so this " + + "configuration would not measure the intended threadpool pressure."); + } + } + + [IterationSetup] + public void IterationSetup() + { + // Warm the pool to MaxPoolSize so the measured run reflects steady-state + // checkout/return rather than first-time physical connection establishment. + WarmPool(MaxPoolSize); + } + + [IterationCleanup] + public void IterationCleanup() + { + using var conn = new SqlConnection(_connectionString); + SqlConnection.ClearPool(conn); + } + + [Benchmark] + public Task SaturatedSyncOpenOnThreadPool() + { + var tasks = new Task[Parallelism]; + for (int i = 0; i < Parallelism; i++) + { + tasks[i] = Task.Run(() => + { + for (int op = 0; op < OpsPerWorker; op++) + { + using var conn = new SqlConnection(_connectionString); + conn.Open(); + using var cmd = conn.CreateCommand(); + cmd.CommandText = "SELECT 1"; + _ = cmd.ExecuteScalar(); + // Dispose returns the connection to the pool. + } + }); + } + + return Task.WhenAll(tasks); + } + + private void WarmPool(int count) + { + var conns = new SqlConnection[count]; + try + { + for (int i = 0; i < count; i++) + { + conns[i] = new SqlConnection(_connectionString); + conns[i].Open(); + } + } + finally + { + // Close and dispose every connection so they return to the pool and + // are not retained until GC (which would add allocation/GC noise). + for (int i = 0; i < count; i++) + { + conns[i]?.Close(); + conns[i]?.Dispose(); + } + } + } + } +} diff --git a/src/Microsoft.Data.SqlClient/tests/PerformanceTests/Config/Config.cs b/src/Microsoft.Data.SqlClient/tests/PerformanceTests/Config/Config.cs index 7b136c6f2c..41d756c200 100644 --- a/src/Microsoft.Data.SqlClient/tests/PerformanceTests/Config/Config.cs +++ b/src/Microsoft.Data.SqlClient/tests/PerformanceTests/Config/Config.cs @@ -71,6 +71,8 @@ public class Benchmarks public RunnerJob ConnectionPoolStressRunnerConfig; public RunnerJob ConnectionPoolContentionRunnerConfig; public RunnerJob ConnectionPoolChurnRunnerConfig; + public RunnerJob ConnectionPoolRampRunnerConfig; + public RunnerJob ConnectionPoolThreadPoolPressureRunnerConfig; } public class RunnerJob diff --git a/src/Microsoft.Data.SqlClient/tests/PerformanceTests/Program.cs b/src/Microsoft.Data.SqlClient/tests/PerformanceTests/Program.cs index b5ef4d76c6..8e305efddd 100644 --- a/src/Microsoft.Data.SqlClient/tests/PerformanceTests/Program.cs +++ b/src/Microsoft.Data.SqlClient/tests/PerformanceTests/Program.cs @@ -55,6 +55,8 @@ public BenchmarkUnit(string name, Func selector, Type run new BenchmarkUnit("ConnectionPoolStress", b => b.ConnectionPoolStressRunnerConfig, typeof(ConnectionPoolStressRunner)), new BenchmarkUnit("ConnectionPoolContention", b => b.ConnectionPoolContentionRunnerConfig, typeof(ConnectionPoolContentionRunner)), new BenchmarkUnit("ConnectionPoolChurn", b => b.ConnectionPoolChurnRunnerConfig, typeof(ConnectionPoolChurnRunner)), + new BenchmarkUnit("ConnectionPoolRamp", b => b.ConnectionPoolRampRunnerConfig, typeof(ConnectionPoolRampRunner)), + new BenchmarkUnit("ConnectionPoolThreadPoolPressure", b => b.ConnectionPoolThreadPoolPressureRunnerConfig, typeof(ConnectionPoolThreadPoolPressureRunner)), }; /// diff --git a/src/Microsoft.Data.SqlClient/tests/PerformanceTests/runnerconfig.jsonc b/src/Microsoft.Data.SqlClient/tests/PerformanceTests/runnerconfig.jsonc index 5df16e3ff5..a7648f0604 100644 --- a/src/Microsoft.Data.SqlClient/tests/PerformanceTests/runnerconfig.jsonc +++ b/src/Microsoft.Data.SqlClient/tests/PerformanceTests/runnerconfig.jsonc @@ -173,6 +173,24 @@ "InvocationCount": 1, "WarmupCount": 1, "RowCount": 0 + }, + "ConnectionPoolRampRunnerConfig": { + "Enabled": true, + "LaunchCount": 1, + "IterationCount": 15, + "InvocationCount": 1, + "WarmupCount": 1, + "RowCount": 0 + }, + // Measures tail latency from threadpool starvation, so it needs more iterations + // than the others: a stall shows up in the distribution, not in the median. + "ConnectionPoolThreadPoolPressureRunnerConfig": { + "Enabled": true, + "LaunchCount": 1, + "IterationCount": 25, + "InvocationCount": 1, + "WarmupCount": 1, + "RowCount": 0 } } } diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs index d647ab0914..5b51d93fcf 100644 --- a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs @@ -235,6 +235,93 @@ out DbConnectionInternal? internalConnection Assert.Equal(pool.PoolGroupOptions.MaxPoolSize, pool.Count); } + /// + /// Verifies that an asynchronous request satisfied by an already-idle connection is + /// completed inline on the caller's thread, rather than being dispatched to the thread pool. + /// + /// + /// The fast path must report completion by returning true with the connection, exactly as + /// WaitHandleDbConnectionPool does on its inline hit. Completing the TaskCompletionSource + /// and returning false would look equivalent but is not: it sends + /// SqlConnection.InternalOpenAsync down its asynchronous branch, which allocates an + /// OpenAsyncRetry and schedules ContinueWith(..., TaskScheduler.Default), costing a thread + /// pool dispatch even though the result is already available. Asserting that the + /// TaskCompletionSource is left untouched is what pins that down. + /// + [Fact] + public void GetConnectionAsync_WithIdleConnection_ShouldCompleteInline() + { + // Arrange: take a connection and return it, leaving one connection idle in the pool. + var pool = ConstructPool(SuccessfulConnectionFactory); + SqlConnection owningConnection = new(); + + pool.TryGetConnection( + owningConnection, + taskCompletionSource: null, + TimeoutTimer.StartNew(TimeSpan.FromSeconds(15)), + out DbConnectionInternal? pooledConnection + ); + Assert.NotNull(pooledConnection); + pool.ReturnInternalConnection(pooledConnection, owningConnection); + + // Act + TaskCompletionSource taskCompletionSource = new(); + var completed = pool.TryGetConnection( + new SqlConnection(), + taskCompletionSource, + TimeoutTimer.StartNew(TimeSpan.FromSeconds(15)), + out DbConnectionInternal? internalConnection + ); + + // Assert: the request is reported as completed and the connection is handed back + // directly, matching WaitHandleDbConnectionPool's inline hit. This is what lets + // SqlConnection.InternalOpenAsync take its synchronous branch and skip the + // OpenAsyncRetry allocation and the ContinueWith thread pool dispatch. + Assert.True(completed); + Assert.Equal(pooledConnection, internalConnection); + + // The TaskCompletionSource must be left alone; the caller abandons it on a + // synchronous completion. + Assert.False(taskCompletionSource.Task.IsCompleted); + + // The idle connection was reused rather than a second one being opened. + Assert.Equal(1, pool.Count); + } + + /// + /// Verifies that a synchronous request satisfied by an already-idle connection returns that + /// connection inline. + /// + [Fact] + public void GetConnection_WithIdleConnection_ShouldReturnInline() + { + // Arrange: take a connection and return it, leaving one connection idle in the pool. + var pool = ConstructPool(SuccessfulConnectionFactory); + SqlConnection owningConnection = new(); + + pool.TryGetConnection( + owningConnection, + taskCompletionSource: null, + TimeoutTimer.StartNew(TimeSpan.FromSeconds(15)), + out DbConnectionInternal? pooledConnection + ); + Assert.NotNull(pooledConnection); + pool.ReturnInternalConnection(pooledConnection, owningConnection); + + // Act + var completed = pool.TryGetConnection( + new SqlConnection(), + taskCompletionSource: null, + TimeoutTimer.StartNew(TimeSpan.FromSeconds(15)), + out DbConnectionInternal? internalConnection + ); + + // Assert + Assert.True(completed); + Assert.Equal(pooledConnection, internalConnection); + Assert.Equal(1, pool.Count); + } + /// /// Verifies that a waiting synchronous caller reuses a connection that is returned to an /// exhausted pool instead of creating a new physical connection. @@ -652,10 +739,19 @@ public void StressTestAsync() TimeoutTimer.StartNew(TimeSpan.FromSeconds(15)), out DbConnectionInternal? internalConnection ); - internalConnection = await taskCompletionSource.Task; - pool.ReturnInternalConnection(internalConnection, owningObject); + + // A request satisfied from the pool's existing connections completes inline, + // returning the connection directly and leaving the TaskCompletionSource + // untouched. Only fall back to awaiting it when the request was handed off. + // This mirrors how the pool's callers consume TryGetConnection. + if (!completed) + { + internalConnection = await taskCompletionSource.Task; + } Assert.NotNull(internalConnection); + + pool.ReturnInternalConnection(internalConnection, owningObject); }); tasks.Add(t); }