Skip to content

Plan analysis: wait stats become findings, and external and preemptive waits get a real benefit (#4516, #4517) - #4555

Merged
erikdarlingdata merged 4 commits into
devfrom
plan-sync/4516-4517-waits
Sep 28, 2026
Merged

erikdarlingdata merged 4 commits into
devfrom
plan-sync/4516-4517-waits

Conversation

@erikdarlingdata

@erikdarlingdata erikdarlingdata commented Sep 28, 2026 •

Copy link
Copy Markdown
Owner

Fixes #4516
Fixes #4517
Part of #4511

Why

BenefitScorer.ScoreWaitStats already computes a benefit percent for each
wait type recorded on a statement in an actual plan, and stores it on
PlanStatement.WaitBenefits. Two gaps sat next to that:

  1. Nothing turned a wait into a finding. A query that spent most of its
    elapsed time waiting on I/O or locks got no warning about it — the wait
    only showed up in the raw wait-stats list, sorted apart from the
    operator findings that share the same benefit scale.
  2. The benefit math itself missed a class of waits. External and
    preemptive waits (MEMORY_ALLOCATION_EXT, RESERVED_MEMORY_ALLOCATION_EXT,
    the PREEMPTIVE_* waits) keep the worker thread CPU-busy in the kernel,
    so its elapsed time is about equal to its CPU time. The existing
    per-thread elapsed - cpu math barely shows those waits at all.

Takes effect once the scorer runs in the product: #4552 (Fixes #4546) wires it in. Nothing in the product calls BenefitScorer.Score before that.

What changes

  • BenefitScorer.EmitWaitStatWarnings runs right after ScoreWaitStats for
    every statement with wait stats, and adds one "Wait: <type>" finding per
    wait (skipping zero-time waits, same as before).
    • The message states the observed wait time and count.
    • Severity comes from the wait's benefit percent: Critical at 50 or more,
      Warning at 10 or more, otherwise Info.
    • PAGEIOLATCH_* findings (and any other wait configured for it) also
      carry the average time per wait (wait time divided by wait count).
  • New WaitStatsConfig reads an embedded Resources/WaitStats.json (44
    wait types, one entry per name) and is the single source of the per-wait
    display flags and any curated description. Most entries have no
    description yet — this ships the mechanism, not the copy; descriptions
    fill in over time.
  • WaitStats.json is an EmbeddedResource in
    PerformanceMonitor.PlanAnalysis.csproj, so Darling, Lite, and the plan
    viewer all read the same copy through their shared project reference — no
    per-app duplicate.
  • New BenefitScorer.IsExternalWait classifies MEMORY_ALLOCATION* and
    PREEMPTIVE_* waits and routes them through a separate formula instead
    of the standard per-thread elapsed - cpu wait math: the wait's share of
    the statement's total CPU, scaled by the sum of each operator's max
    per-thread self-CPU.
  • New PlanAnalyzer.GetOperatorMaxThreadOwnCpuMs, sitting next to the
    existing GetOperatorOwnElapsedMs, gives that per-operator max
    per-thread self-CPU (non-cumulative, so summing across operators in a
    serial plan doesn't double-count).
  • No AnalyzerConfig rule-disable guard: this project doesn't have one.

Not ported

  • PerformanceStudio's AdviceContentBuilder inline wait label (a short
    "I/O — reading from disk" style tag next to the wait-stats card) is
    UI-side display code with no equivalent surface in this project's
    PlanAnalysis library. The finding message carries the curated
    description instead, when one exists in the JSON.
  • PerformanceStudio also adjusts a CPU:Elapsed ratio (web runtime card,
    HTML export runtime card, DOP efficiency) by subtracting external-wait
    time from CPU. This project has no equivalent runtime-card surface today,
    so that half of the source commit isn't ported; only the benefit-scoring
    half (IsExternalWait, the external-wait formula, and
    GetOperatorMaxThreadOwnCpuMs) is.

Consumers and impact

Once #4552 runs the scorer, the "Wait: " findings reach the plan viewer, the MCP plan tools and Darling's stored plan advisories. Measured on 60 real showplans, they add 77 Info, 1 Warning and 11 Critical findings. In Darling's analysis they reach only the PLAN_WARNING fact (scored presence-only, 0.4, below the Warning band and the notification threshold) and the advice headline's critical count, so no alert or health band moves.

Test plan

  • Darling.Tests.PlanSync4516Tests (9 tests) — pins the severity tiers
    (including the 10% and 50% boundaries), the PAGEIOLATCH_* average-latency
    line, the curated-description round trip through WaitStatsConfig, an
    absent-wait miss, the embedded resource loading a real wait type, and one
    pin through the full ShowPlanParser.Parse → PlanAnalyzer.Analyze →
    BenefitScorer.Score pipeline.
  • Darling.Tests.PlanSync4517Tests (11 tests) — IsExternalWait's prefix
    matching, PerformanceStudio's worked example (48.3%), a
    PREEMPTIVE_OS_WRITEFILEGATHER wait on a CPU-busy thread scoring well
    above the old formula's result on the same inputs, an ordinary wait
    taking the unchanged old route, GetOperatorMaxThreadOwnCpuMs's serial
    self-CPU subtraction, and one pin through the full parse → analyze →
    score pipeline.
  • Darling.Tests.PlanSync4517ProbeTests (1 test) — the runtime RED that
    was previously recorded compile-only. It builds only the pre-existing
    members (PlanAnalyzer.Analyze, BenefitScorer.Score) that already
    ship on dev, feeds them the same CPU-busy-thread shape as The benefit for external and preemptive waits is far too low (MEMORY_ALLOCATION_EXT, PREEMPTIVE_*) #4517's
    worked example, and asserts the new 90.2% CPU-share answer. Run
    against a detached origin/dev worktree (pre-The benefit for external and preemptive waits is far too low (MEMORY_ALLOCATION_EXT, PREEMPTIVE_*) #4517), it fails:
    Assert.Equal() Failure: Values differ / Expected: 90.200000000000003 / Actual: 50 — the old per-thread elapsed-minus-cpu formula's answer for
    that same shape. On this branch it's green.
  • Run: every PlanSync* class plus the plan-analysis classes passed. DarlingAnalysisPipelineTests and DarlingMcpPlanToolsLivePostgresTests need a live store and run in CI.
  • Mutations, each reverted: removing the EmitWaitStatWarnings call fails 5 of the 9 Wait stats never become findings: BenefitScorer computes a benefit for each wait but emits no warning #4516 facts; IsExternalWait returning false fails 6 of the 11 The benefit for external and preemptive waits is far too low (MEMORY_ALLOCATION_EXT, PREEMPTIVE_*) #4517 facts.
  • Aligned with PerformanceStudio main: GetOperatorMaxThreadOwnCpuMs skips the coordinator thread and looks through batch mode zones and Compute Scalar pass-throughs, as PerformanceStudio's shared per-thread helper does.
  • Lite.Tests (Release, -p:EnableWindowsTargeting=true): 0 errors,
    builds only (net10.0-windows can't run on macOS). No Lite.Tests class
    references BenefitScorer, WaitStatsConfig, or
    GetOperatorMaxThreadOwnCpuMs today, so none needed updating.

CHANGELOG

SECTION: Added
ENTRY:

Each wait recorded on a statement in an actual plan now emits a "Wait: <type>"
finding, sitting alongside the existing per-operator findings so it sorts by
benefit percent instead of only living in the raw wait-stats list.

- BenefitScorer emits one finding per wait after it scores the wait's benefit
  percent. Severity comes from that percent: Critical at 50 or more, Warning
  at 10 or more, otherwise Info.
- PAGEIOLATCH_* findings also carry the average time per wait (wait time
  divided by wait count).
- Per-wait display flags and curated descriptions come from an embedded
  WaitStats.json read through WaitStatsConfig, so Darling, Lite, and the
  viewer share one copy through the shared PlanAnalysis project. Most entries
  have no description yet; that fills in over time.

Refs #4511
MEMORY_ALLOCATION* and PREEMPTIVE_* waits keep the worker CPU-busy in the kernel, so elapsed is about equal to CPU for those threads and the standard elapsed-minus-cpu wait math barely scores them.

BenefitScorer.IsExternalWait classifies those waits and routes them through a separate formula: the wait's share of statement CPU, scaled by the sum of each operator's max per-thread self-CPU (PlanAnalyzer.GetOperatorMaxThreadOwnCpuMs, next to GetOperatorOwnElapsedMs).

Refs #4517. Part of #4511.
# Conflicts:
#	PerformanceMonitor.PlanAnalysis/PlanAnalyzer.cs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant