diff --git a/eng/pipelines/perf/README.md b/eng/pipelines/perf/README.md index 09ad48788f..42cef42d4a 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,109 @@ 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.RapidFireOpenClose` calls +`ClearAllPools()` in `[IterationCleanup]` and holds connections for zero time, so it measures a +cold-start burst in which the extra parallel opens have nothing to amortise against. It reports the +trade-off as a loss because that is the only thing it can measure. + +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 add a benchmark 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 is the case `RapidFireOpenClose` cannot express. + +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 +266,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** @@ -323,6 +430,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 +450,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 7ea2bfdffe..bf740e61eb 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" @@ -125,6 +135,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)" @@ -138,6 +149,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)." } @@ -299,20 +325,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)) { @@ -321,11 +338,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. @@ -685,6 +742,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 @@ -701,8 +763,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, @@ -714,6 +785,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" @@ -723,7 +800,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 4df2eec8e3..6b1ab3a5c7 100755 --- a/eng/pipelines/perf/scripts/run-perf-tests.sh +++ b/eng/pipelines/perf/scripts/run-perf-tests.sh @@ -62,10 +62,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]" \ @@ -80,6 +92,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 ;; @@ -100,12 +113,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 @@ -120,6 +147,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 @@ -145,6 +184,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:-}" @@ -292,7 +332,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). @@ -355,10 +397,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() @@ -384,13 +436,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") @@ -399,6 +454,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. @@ -683,6 +753,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 @@ -699,12 +774,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}" @@ -712,6 +799,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) @@ -721,7 +817,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 9f75c50c1a..e5b543e60c 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 @@ -850,11 +850,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 @@ -909,6 +921,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) @@ -1264,6 +1303,73 @@ 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; + } + + PrepareConnection(owningConnection, connection, transaction); + return connection; + } + /// /// Gets an internal connection from the pool, either by retrieving an idle connection or opening a new one. /// diff --git a/src/Microsoft.Data.SqlClient/tests/PerformanceTests/BenchmarkRunners/ConnectionPoolContentionRunner.cs b/src/Microsoft.Data.SqlClient/tests/PerformanceTests/BenchmarkRunners/ConnectionPoolContentionRunner.cs index 899d25f64c..e99875bb92 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 @@ -114,6 +122,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..2c5d172c71 --- /dev/null +++ b/src/Microsoft.Data.SqlClient/tests/PerformanceTests/BenchmarkRunners/ConnectionPoolRampRunner.cs @@ -0,0 +1,150 @@ +// 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)] + public int Parallelism { get; set; } + + /// + /// Max pool size. Deliberately larger than every value so + /// the ramp is never bounded by pool capacity. + /// + [Params(100)] + public int MaxPoolSize { get; set; } + + private string _connectionString; + + [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); + await conn.OpenAsync(); + + // Hold the connection until every caller has one, forcing the pool to + // grow to Parallelism physical connections. + if (allConnected.Signal()) + { + release.TrySetResult(true); + } + + await release.Task; + // 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); + conn.Open(); + + allConnected.Signal(); + allConnected.Wait(); + // Dispose returns the connection to the pool. + }, TaskCreationOptions.LongRunning); + } + + Task.WaitAll(tasks); + } + } +} 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..2dd12490fd --- /dev/null +++ b/src/Microsoft.Data.SqlClient/tests/PerformanceTests/BenchmarkRunners/ConnectionPoolThreadPoolPressureRunner.cs @@ -0,0 +1,172 @@ +// 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 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 rate limited to + /// roughly one or two threads per second, so each stall costs about a second. + /// + /// 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 a statement about application configuration, not a pool defect. + /// An application that blocks more thread pool threads than the thread pool has workers + /// is already misconfigured, and pre-warming the thread pool is the application's + /// responsibility rather than the driver's. 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. The + /// low value is below (starved); the high value is above + /// it (control). + /// + [Params(8, 128)] + public int MinWorkerThreads { get; set; } + + /// + /// 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); + ThreadPool.SetMinThreads(MinWorkerThreads, _originalMinCompletionPortThreads); + + var builder = new SqlConnectionStringBuilder(s_config.ConnectionString) + { + Pooling = true, + MaxPoolSize = MaxPoolSize, + MinPoolSize = 0 + }; + _connectionString = builder.ConnectionString; + } + + [GlobalCleanup] + public void Cleanup() + { + ThreadPool.SetMinThreads( + _originalMinWorkerThreads, _originalMinCompletionPortThreads); + } + + [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 2bbd1f1c23..6afb2067f2 100644 --- a/src/Microsoft.Data.SqlClient/tests/PerformanceTests/Config/Config.cs +++ b/src/Microsoft.Data.SqlClient/tests/PerformanceTests/Config/Config.cs @@ -70,6 +70,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 bf42ebaf75..8fd925cdf3 100644 --- a/src/Microsoft.Data.SqlClient/tests/PerformanceTests/Program.cs +++ b/src/Microsoft.Data.SqlClient/tests/PerformanceTests/Program.cs @@ -54,6 +54,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 5777db9652..d9088af050 100644 --- a/src/Microsoft.Data.SqlClient/tests/PerformanceTests/runnerconfig.jsonc +++ b/src/Microsoft.Data.SqlClient/tests/PerformanceTests/runnerconfig.jsonc @@ -142,6 +142,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 228e949cd0..2cd8867345 100644 --- a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs @@ -233,6 +233,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. @@ -626,10 +713,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); }