Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
erikdarlingdata merged 1 commit into from
Apr 22, 2026
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 11 additions & 3 deletions src/PlanViewer.App/Controls/PlanViewerControl.axaml.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2800,9 +2800,12 @@ void AddRow(string label, string value, string? color = null)
rowIndex++;
}

// Efficiency thresholds: white >= 80%, yellow >= 60%, orange >= 40%, red < 40%
static string EfficiencyColor(double pct) => pct >= 80 ? "#E4E6EB"
: pct >= 60 ? "#FFD700" : pct >= 40 ? "#FFB347" : "#E57373";
// 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";
Comment on lines +2803 to +2808
Copy link
Copy Markdown
Owner Author

@erikdarlingdata erikdarlingdata Apr 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


// Runtime stats (actual plans)
if (statement.QueryTimeStats != null)
Expand All @@ -2815,6 +2818,11 @@ static string EfficiencyColor(double pct) => pct >= 80 ? "#E4E6EB"
AddRow("UDF elapsed", $"{statement.QueryUdfElapsedTimeMs:N0}ms");
}

// 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");
Copy link
Copy Markdown
Owner Author

@erikdarlingdata erikdarlingdata Apr 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


// Memory grant — color by utilization percentage
if (statement.MemoryGrant != null)
{
Expand Down
2 changes: 1 addition & 1 deletion src/PlanViewer.App/PlanViewer.App.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
<ApplicationManifest>app.manifest</ApplicationManifest>
<ApplicationIcon>EDD.ico</ApplicationIcon>
<AvaloniaUseCompiledBindingsByDefault>true</AvaloniaUseCompiledBindingsByDefault>
<Version>1.7.4</Version>
<Version>1.7.5</Version>
<Authors>Erik Darling</Authors>
<Company>Darling Data LLC</Company>
<Product>Performance Studio</Product>
Expand Down
4 changes: 3 additions & 1 deletion src/PlanViewer.Core/Output/HtmlExporter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -308,14 +308,16 @@ private static void WriteRuntimeCard(StringBuilder sb, StatementResult stmt)
WriteRow(sb, "CPU:Elapsed", ratio.ToString("N2"));
}
}
if (stmt.CompileTimeMs > 0)
WriteRow(sb, "Compile", $"{stmt.CompileTimeMs:N0} ms");
if (stmt.DegreeOfParallelism > 0)
WriteRow(sb, "DOP", stmt.DegreeOfParallelism.ToString());
if (stmt.NonParallelReason != null)
WriteRow(sb, "Serial", Encode(stmt.NonParallelReason));
if (stmt.MemoryGrant != null && stmt.MemoryGrant.GrantedKB > 0)
{
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";
Copy link
Copy Markdown
Owner Author

@erikdarlingdata erikdarlingdata Apr 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

WriteRow(sb, "Memory", FormatKB(stmt.MemoryGrant.GrantedKB) + " granted");
sb.AppendLine($"<div class=\"row\"><span class=\"label\">Used</span><span class=\"value {effClass}\">{FormatKB(stmt.MemoryGrant.MaxUsedKB)} ({pctUsed:N0}%)</span></div>");
}
Expand Down
9 changes: 8 additions & 1 deletion src/PlanViewer.Web/Pages/Index.razor
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,13 @@ else
</div>
}
}
@if (ActiveStmt!.CompileTimeMs > 0)
{
<div class="insight-row">
<span class="insight-label">Compile</span>
<span class="insight-value">@ActiveStmt!.CompileTimeMs.ToString("N0") ms</span>
</div>
}
@if (ActiveStmt!.DegreeOfParallelism > 0)
{
<div class="insight-row">
Expand All @@ -188,7 +195,7 @@ else
@if (ActiveStmt!.MemoryGrant != null && ActiveStmt!.MemoryGrant.GrantedKB > 0)
{
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";
Copy link
Copy Markdown
Owner Author

@erikdarlingdata erikdarlingdata Apr 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

<div class="insight-row">
<span class="insight-label">Memory</span>
<span class="insight-value">@FormatKB(ActiveStmt!.MemoryGrant.GrantedKB) granted</span>
Expand Down
Loading