Loosen memory grant colors + compile time as plan-level property (#215 C1, C4) by erikdarlingdata · Pull Request #261 · erikdarlingdata/PerformanceStudio

erikdarlingdata · 2026-04-22T16:32:58Z

Summary

C1: memory grant color scale was too harsh (61% utilized showed orange). New tiers: ≥40% good, 20–39% warn, <20% bad. Operators spill near their max, so moderate utilization is fine; what we flag now is real over-granting.
C4: compile time always visible in the Runtime panel (category B plan-level property), not just when Rule 19 fires above 1000ms.

Details

`PlanViewerControl.EfficiencyColor` loosened — affects memory grant, DOP efficiency, thread utilization consistently.
`HtmlExporter` memory card eff class + `Index.razor` eff class updated.
`Compile: Nms` row added to Runtime card on all three surfaces.
Version 1.7.4 → 1.7.5.

Test plan

Build green
Visual: 61% grant now reads green; compile row visible on plans with >0 compile time

…anel (#215 C1, C4) C1: memory grant utilization color was too harsh. Old thresholds said anything under 80% used was warn/bad, so a 61% grant showed orange. Joe's point: operators spill near their max grant, so moderate utilization is fine and even preferable; what we actually want to flag is significant over-granting (very low utilization, reserved memory wasted). New thresholds: >= 40% utilized: good (neutral / green) 20-39%: warn (orange) < 20%: bad (red) Applied in three places: - PlanViewerControl.EfficiencyColor (used for memory grant, DOP efficiency, and thread utilization — loosens all three consistently) - HtmlExporter memory card eff class - Index.razor memory card eff class C4: compile time is now shown as a plan-level property in the Runtime panel of all three surfaces regardless of whether Rule 19 fires, per Joe's point that compile time belongs in the category-B "plan-level" grouping. Previously only visible when compile CPU >= 1000ms via the warning. Small `Compile: Nms` row next to Elapsed/CPU/DOP. Version bump 1.7.4 -> 1.7.5. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

erikdarlingdata

Review summary

What it does

Loosens the efficiency color ramp: good >=80% / warn >=60% / orange >=40% / bad becomes good >=40% / orange >=20% / bad across three surfaces (App PlanViewerControl, HtmlExporter, Web Index.razor).
Adds a Compile row to the Runtime panel / card on all three surfaces so compile time shows whenever CompileTimeMs > 0.
Bumps version 1.7.4 → 1.7.5.

Good

Base branch is dev.
New colors (#E4E6EB, #FFB347, #E57373) stay clear of the rejected dim-grey range.
Pure code-behind, no MVVM creep.
No PlanAnalyzer rule/scoring changes, so PerformanceMonitor Dashboard/Lite don't need a sibling sync for this PR.

Needs attention

Duplicate compile row in PlanViewer.App/Controls/PlanViewerControl.axaml.cs — the new AddRow("Compile", …) at 2823-2824 sits in the same BuildRuntimeSummary method as the existing AddRow("Compile time", …) at 2895-2896. Users will see two near-identical rows. Delete one. (See inline.)
Shared EfficiencyColor side effects at PlanViewerControl.axaml.cs:2803-2808 — the loosened thresholds also apply to DOP parallel efficiency (2862) and thread utilization (2878), not just memory grants. PR description frames this as memory-only; confirm the parallelism/thread coloring change is intended. (See inline.)
Dead CSS — .eff-ok in src/PlanViewer.Web/wwwroot/css/app.css:618 has no remaining references after this PR. Remove in the same change to keep surfaces consistent.
No test updates — thresholds are display-only so it's borderline, but tests/PlanViewer.Core.Tests/ already covers memory grant rules (e.g. Rule09a_*, LargeMemoryGrant_*). A cheap regression test against the new ramp in HtmlExporterTests.cs would lock the 40/20 boundary in.

Generated by Claude Code

erikdarlingdata · 2026-04-22T16:35:04Z

+        // Compile time — plan-level property (category B). Show regardless of
+        // threshold so it's always visible, not just when Rule 19 fires.
+        if (statement.CompileTimeMs > 0)
+            AddRow("Compile", $"{statement.CompileTimeMs:N0}ms");


Duplicate row. The same BuildRuntimeSummary method already emits compile time unconditionally at line 2895-2896:

// Compile stats (always available) if (statement.CompileTimeMs > 0) AddRow("Compile time", $"{statement.CompileTimeMs:N0}ms");

With this change, the Runtime panel will render two near-identical rows — Compile near the top and Compile time near the bottom. The rationale in the comment above ("Show regardless of threshold so it's always visible, not just when Rule 19 fires") doesn't match the existing code — this panel never gated compile time on Rule 19 firing. Pick one placement and delete the other; presumably the intent is to keep this new early placement next to the Runtime stats block and remove line 2895-2896.

Generated by Claude Code

erikdarlingdata · 2026-04-22T16:35:12Z

+        // Efficiency thresholds: white >= 40%, orange >= 20%, red < 20%.
+        // Loosened per Joe's feedback (#215 C1): for memory grants, moderate
+        // utilization (e.g. 60%) is fine — operators can spill near their max,
+        // so we shouldn't flag anything above a real over-grant threshold.
+        static string EfficiencyColor(double pct) => pct >= 40 ? "#E4E6EB"
+            : pct >= 20 ? "#FFB347" : "#E57373";


EfficiencyColor is also called for DOP parallel efficiency (line 2862) and thread utilization (line 2878), so this threshold shift isn't memory-grant-only. Under the old scheme a plan running at ~50% parallel efficiency was yellow; now it's white/"good". Same for thread utilization. If that's intentional, the comment should say so; if not, you'll want a separate threshold for the parallelism/thread callers. Worth confirming before merge since the PR description frames this as a memory-grant tweak.

Generated by Claude Code

erikdarlingdata · 2026-04-22T16:35:20Z

                {
                    var pctUsed = (double)ActiveStmt!.MemoryGrant.MaxUsedKB / ActiveStmt!.MemoryGrant.GrantedKB * 100;
-                    var effClass = pctUsed >= 80 ? "eff-good" : pctUsed >= 60 ? "eff-ok" : pctUsed >= 40 ? "eff-warn" : "eff-bad";
+                    var effClass = pctUsed >= 40 ? "eff-good" : pctUsed >= 20 ? "eff-warn" : "eff-bad";


The eff-ok tier is dropped here but .eff-ok { color: #f9a825; } still lives in src/PlanViewer.Web/wwwroot/css/app.css:618. No references remain after this change — safe to delete the CSS rule in the same PR to keep the three surfaces aligned.

Generated by Claude Code

erikdarlingdata · 2026-04-22T16:35:33Z

        {
            var pctUsed = (double)stmt.MemoryGrant.MaxUsedKB / stmt.MemoryGrant.GrantedKB * 100;
-            var effClass = pctUsed >= 80 ? "eff-good" : pctUsed >= 40 ? "eff-warn" : "eff-bad";
+            var effClass = pctUsed >= 40 ? "eff-good" : pctUsed >= 20 ? "eff-warn" : "eff-bad";


Threshold shift here is isolated to the memory grant card, so this one reads cleanly — just noting for the record that HtmlExporter only applies the new thresholds to memory (unlike the App control, where the shared EfficiencyColor also affects DOP/thread). Three surfaces now behave slightly differently for parallelism coloring (App uses 40/20, HTML doesn't color DOP, Web doesn't color DOP). Probably fine but worth being explicit about.

Generated by Claude Code

erikdarlingdata commented Apr 22, 2026

View reviewed changes

erikdarlingdata merged commit 5731ae9 into dev Apr 22, 2026
2 checks passed

erikdarlingdata deleted the fix/c1-memory-color-c4-compile-time branch April 22, 2026 16:37

erikdarlingdata mentioned this pull request Apr 22, 2026

Release v1.7.6 — C1 memory color + C4 compile time + C8 expensive-op + C9 columnstore #264

Merged

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Loosen memory grant colors + compile time as plan-level property (#215 C1, C4)#261

Loosen memory grant colors + compile time as plan-level property (#215 C1, C4)#261
erikdarlingdata merged 1 commit intodevfrom
fix/c1-memory-color-c4-compile-time

erikdarlingdata commented Apr 22, 2026

Uh oh!

erikdarlingdata left a comment

Uh oh!

erikdarlingdata Apr 22, 2026

Uh oh!

erikdarlingdata Apr 22, 2026

Uh oh!

erikdarlingdata Apr 22, 2026

Uh oh!

erikdarlingdata Apr 22, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

erikdarlingdata commented Apr 22, 2026

Summary

Details

Test plan

Uh oh!

erikdarlingdata left a comment

Choose a reason for hiding this comment

Review summary

Uh oh!

erikdarlingdata Apr 22, 2026

Choose a reason for hiding this comment

Uh oh!

erikdarlingdata Apr 22, 2026

Choose a reason for hiding this comment

Uh oh!

erikdarlingdata Apr 22, 2026

Choose a reason for hiding this comment

Uh oh!

erikdarlingdata Apr 22, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant