Wait stats as warnings (#215, Joe feedback a1/b4/b5) by erikdarlingdata · Pull Request #244 · erikdarlingdata/PerformanceStudio

erikdarlingdata · 2026-04-21T00:19:26Z

Summary

Each recorded wait stat emits a first-class PlanWarning with per-wait description, how-to-fix guidance, and the benefit % already calculated by BenefitScorer. Joe's a1/b4/b5 asks from [FEATURE] Add system to assign a "maximum benefit" for plan analysis rules #215 — integrate waits into the warnings list instead of leaving them in their own card.
Wait-stats chart stays as a complementary view (editorial call from the issue thread).

Details

New: `WaitStatsKnowledge.cs`

Static content library mapping wait type → description + how-to-fix. Covers:

I/O: PAGEIOLATCH_SH/EX/UP/DT, WRITELOG, IO_COMPLETION, ASYNC_IO_COMPLETION, LOGBUFFER
Memory: MEMORY_ALLOCATION_EXT, RESOURCE_SEMAPHORE, RESOURCE_SEMAPHORE_QUERY_COMPILE, SOS_PHYS_PAGE_CACHE
Scheduler: SOS_SCHEDULER_YIELD, THREADPOOL, DISPATCHER_QUEUE_SEMAPHORE
Parallelism: CXPACKET, CXCONSUMER, CXSYNC_PORT, CXSYNC_CONSUMER
Batch-mode hash: HTBUILD, HTREPARTITION, HTDELETE, HTMEMO, HTREINIT, BPSORT, BMPBUILD
Latch: PAGELATCH_SH/EX/UP, LATCH_EX, LATCH_SH
Lock: LCK_M_S/X/U/IS/IX/SCH_S/SCH_M/RS_S/RS_U/RX_X
Network: ASYNC_NETWORK_IO
Misc: EXECSYNC, PREEMPTIVE_OS_WRITEFILEGATHER

Prefix fallbacks for LCK_M_, PAGEIOLATCH_, PAGELATCH_, LATCH_, HT, CX, PREEMPTIVE_, plus a MEMORY_ALLOCATION contains-match (catches RESERVED_MEMORY_ALLOCATION_EXT).

`BenefitScorer.EmitWaitStatWarnings`

Runs after ScoreWaitStats. For each WaitStatInfo it builds a warning:

WarningType = "Wait: {WaitType}"
Message = {WaitType}: {description}. Observed N ms across M waits.
For PAGEIOLATCH_* also appends Effective latency: X ms/µs per wait (wait_ms / wait_count)
MaxBenefitPercent from the existing wait-benefit calculation
ActionableFix = how-to-fix from the knowledge library
Severity tier from benefit %: >=50 Critical, >=10 Warning, else Info

Renderers

No changes needed — warnings flow through stmt.PlanWarnings → ResultMapper → web viewer, HTML export, JSON/text, Avalonia Plan Warnings expander.

Version bump

1.6.0 → 1.7.0.

Test plan

CLI smoke on .internal/examples/20260415_1.sqlplan — CXPACKET 99% Critical, ASYNC_NETWORK_IO 93% Critical, HT*/LATCH_EX/MEMORY_ALLOCATION_EXT as Info
CLI smoke on .internal/examples/NXfso4jC0L.sqlplan — PAGEIOLATCH_SH shows Effective latency: 102 µs per wait, MEMORY_ALLOCATION_EXT 77.7% Critical, RESERVED_MEMORY_ALLOCATION_EXT picked up by fallback
Visual check on web viewer after merge
Visual check on Avalonia desktop after merge

🤖 Generated with Claude Code

Each wait stat recorded on a statement now emits a first-class PlanWarning alongside the existing wait-stats card, so waits sort by benefit % in the warnings list next to operator findings instead of living only in their own panel. - WaitStatsKnowledge: per-wait description + how-to-fix content covering PAGEIOLATCH_*, PAGELATCH_*, LATCH_*, LCK_M_*, MEMORY_ALLOCATION_EXT, RESOURCE_SEMAPHORE*, CX*/HT*/BMPBUILD parallel + batch-mode families, SOS_SCHEDULER_YIELD, THREADPOOL, ASYNC_NETWORK_IO, WRITELOG, IO_COMPLETION, LOGBUFFER, EXECSYNC, PREEMPTIVE_OS_WRITEFILEGATHER, with prefix fallbacks. - PAGEIOLATCH_SH/EX/UP/DT warnings include effective latency per wait (wait_ms / wait_count) in the message. - Severity tiers off benefit %: >=50 Critical, >=10 Warning, else Info. - Flows through existing PlanWarnings pipeline so web viewer, HTML export, JSON/text, and Avalonia Plan Warnings expander all pick it up with no renderer changes. Wait-stats chart stays as a complementary view. - Version bump 1.6.0 -> 1.7.0. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

erikdarlingdata

Review: PR #244 — Wait stats as warnings

What this does

Adds WaitStatsKnowledge (a static wait-type → description/fix/latency-hint table) and BenefitScorer.EmitWaitStatWarnings, which emits one PlanWarning per wait stat entry after ScoreWaitStats runs. Severity tiers from the existing benefit % (>=50 Critical, >=10 Warning, else Info). Wires the new file into PlanViewer.Web.csproj's compile-link list and bumps PlanViewer.App to 1.7.0.

What's good

Base branch is dev — correct.
No Avalonia/XAML changes, so none of the usual Avalonia gotchas (AvaloniaEdit style include, PlaceholderText vs Watermark, :pointerover /template/ selectors, MVVM creep) apply here.
No new brushes or inline colors, so no text-contrast concerns.
App and Cli pick up the new file via ProjectReference; Web correctly got an explicit <Compile Include> entry.
FormatLatency picks reasonable units across the range.
Knowledge-library content is well-curated with a sensible default fallback.

What needs attention

Cross-repo sync — scoring change. This touches BenefitScorer, which is Studio's plan-analysis/scoring surface. The equivalent wait-as-warning behavior (or an explicit decision to skip it) needs to land in PerformanceMonitor's Dashboard and Lite copies to keep the rule set in sync. Not blocking this PR, but file a tracking task.
Test coverage. WaitStatsKnowledge.Lookup has non-trivial branching (exact → 7 prefix fallbacks → Contains("MEMORY_ALLOCATION") → default) and EmitWaitStatWarnings has severity tiering, latency-hint formatting, and the guard-asymmetry case flagged inline. Nothing in tests/PlanViewer.Core.Tests/ covers any of it. At minimum:
- Lookup returns exact for known keys, the right prefix entry for unknowns in each family, Default for ""/null/unknowns with no matching prefix.
- EmitWaitStatWarnings severity tiers at the 50 / 10 / below-10 boundaries.
- Effective-latency suffix appears only for entries with ShowEffectiveLatency = true and WaitCount > 0.
Inline findings (see comments on the diff):
- BenefitScorer.cs:43 — guard asymmetry: EmitWaitStatWarnings runs when QueryTimeStats is null, so benefit % is always missing and everything is Info.
- BenefitScorer.cs:91 — WarningType = "Wait: " + wait.WaitType produces unbounded distinct warning types; confirm no downstream switch/grouping breaks.
- WaitStatsKnowledge.cs:265 — StartsWith("HT") is overly broad as a family fallback.
- WaitStatsKnowledge.cs:286 — Exact["MEMORY_ALLOCATION_EXT"] throws if the key is ever renamed; use TryGetValue or inline the entry.

No approval / no changes requested — leaving the call to the maintainer.

Generated by Claude Code

erikdarlingdata · 2026-04-21T00:21:12Z

                    ScoreWaitStats(stmt);
+
+                if (stmt.WaitStats.Count > 0)
+                    EmitWaitStatWarnings(stmt);


Guard asymmetry worth confirming: ScoreWaitStats on line 40 only runs when stmt.QueryTimeStats != null, but EmitWaitStatWarnings on line 43 runs whenever WaitStats.Count > 0.

When QueryTimeStats is null (e.g. estimated-only plans with recorded wait stats), stmt.WaitBenefits stays empty, so benefitByType.TryGetValue always misses and every wait is emitted at Info severity with MaxBenefitPercent = null. The plan may have a real 99%-elapsed CXPACKET problem that silently shows up as Info.

Intentional degrade? If so, fine — but it's worth a comment at line 42 saying so. Otherwise consider matching the QueryTimeStats != null guard.

Generated by Claude Code

erikdarlingdata · 2026-04-21T00:21:17Z

+                Description = "Waiting to read a data page from disk into the buffer pool.",
+                HowToFix = "Reduce physical reads (better indexes, fewer pages touched, fix cardinality estimates) or move to faster storage.",
+                ShowEffectiveLatency = true
+            };


StartsWith("HT") is overly broad — any future or unrelated wait name that happens to begin with HT gets labeled as a batch-mode hash synchronization wait. The known batch-mode HT* waits (HTBUILD, HTREPARTITION, HTDELETE, HTMEMO, HTREINIT) are already in Exact, so this prefix fallback only fires on unknowns.

Consider tightening to a known-prefix allowlist (e.g. check HTBUILD/HTREPARTITION/HTDELETE/HTMEMO/HTREINIT via Exact only) or narrowing the prefix to something more specific. Same concern applies to StartsWith("CX") on line 272 — less likely to misfire since real CX* waits are all exchange-related, but the same shape of issue.

Generated by Claude Code

erikdarlingdata · 2026-04-21T00:21:21Z

+            {
+                Description = "Batch-mode hash operation synchronization wait.",
+                HowToFix = "Reduce input rows and skew for the batch-mode hash. Confirm the smaller side is on the build, and check that batch mode is the right choice for this plan shape."
+            };


Exact["MEMORY_ALLOCATION_EXT"] uses the indexer, which throws KeyNotFoundException if someone later renames or removes that exact key. Cheap fix: return Exact.TryGetValue("MEMORY_ALLOCATION_EXT", out var m) ? m : Default; — or just inline the Entry literal here so the fallback doesn't secretly depend on another dictionary entry existing.

Generated by Claude Code

erikdarlingdata · 2026-04-21T00:21:26Z

+
+            stmt.PlanWarnings.Add(new PlanWarning
+            {
+                WarningType = "Wait: " + wait.WaitType,


WarningType = "Wait: " + wait.WaitType creates one unique WarningType per wait type (e.g. "Wait: CXPACKET", "Wait: PAGEIOLATCH_SH"). Any downstream code keying on WarningType — filters, grouping in the web viewer / HTML export / Avalonia Plan Warnings expander, rule-count metrics — now sees an unbounded set of warning types instead of a single "Wait Statistics" bucket. Worth a quick scan of the ResultMapper / exporters / Avalonia warnings view to confirm none of them switch/dictionary on WarningType.

Generated by Claude Code

erikdarlingdata commented Apr 21, 2026

View reviewed changes

erikdarlingdata merged commit 1391589 into dev Apr 21, 2026
2 checks passed

erikdarlingdata deleted the feature/wait-stats-as-warnings branch April 21, 2026 00:23

erikdarlingdata mentioned this pull request Apr 21, 2026

Release v1.7.0 — wait stats as warnings #245

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Wait stats as warnings (#215, Joe feedback a1/b4/b5)#244

Wait stats as warnings (#215, Joe feedback a1/b4/b5)#244
erikdarlingdata merged 1 commit intodevfrom
feature/wait-stats-as-warnings

erikdarlingdata commented Apr 21, 2026

Uh oh!

erikdarlingdata left a comment

Uh oh!

erikdarlingdata Apr 21, 2026

Uh oh!

erikdarlingdata Apr 21, 2026

Uh oh!

erikdarlingdata Apr 21, 2026

Uh oh!

erikdarlingdata Apr 21, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

erikdarlingdata commented Apr 21, 2026

Summary

Details

New: WaitStatsKnowledge.cs

BenefitScorer.EmitWaitStatWarnings

Renderers

Version bump

Test plan

Uh oh!

erikdarlingdata left a comment

Choose a reason for hiding this comment

Review: PR #244 — Wait stats as warnings

What this does

What's good

What needs attention

Uh oh!

erikdarlingdata Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

erikdarlingdata Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

erikdarlingdata Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

erikdarlingdata Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

New: `WaitStatsKnowledge.cs`

`BenefitScorer.EmitWaitStatWarnings`