From 5c45d0bd1e090d391e61e9d9441c2e8763731b53 Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Tue, 29 Sep 2026 17:29:25 -0400 Subject: [PATCH] Plan viewer: CE model sits above Optimization, and Early abort nests under it, as in PerformanceStudio (#4836) --- .../PlanViewerRuntimeSummaryNestingTests.cs | 145 ++++++++++++++++++ Darling/Darling.Tests/Viewer4570Tests.cs | 41 ++++- .../PlanDisplayText.cs | 22 ++- .../PlanViewerControl.Properties.cs | 9 +- 4 files changed, 204 insertions(+), 13 deletions(-) create mode 100644 Darling/Darling.Tests/PlanViewerRuntimeSummaryNestingTests.cs diff --git a/Darling/Darling.Tests/PlanViewerRuntimeSummaryNestingTests.cs b/Darling/Darling.Tests/PlanViewerRuntimeSummaryNestingTests.cs new file mode 100644 index 000000000..dd0fc9b10 --- /dev/null +++ b/Darling/Darling.Tests/PlanViewerRuntimeSummaryNestingTests.cs @@ -0,0 +1,145 @@ +/* + * Copyright (c) 2026 Erik Darling, Darling Data LLC + * + * This file is part of the SQL Server Performance Monitor. + * + * Licensed under the MIT License. See LICENSE file in the project root for full license information. + */ + +using System; +using System.Linq; +using System.Reflection; +using System.Threading; +using System.Windows; +using System.Windows.Controls; +using PerformanceMonitor.PlanAnalysis; +using PerformanceMonitor.Ui; +using Xunit; + +namespace Darling.Tests; + +/// +/// #4836: the Runtime Summary card nests the early abort reason under Optimization, as +/// erikdarlingdata/PerformanceStudio#614 does. The nested row's LABEL gets a 12px left indent and its VALUE +/// stays in the same column as every other value. pins which row is nested (the +/// pure row model, which runs anywhere); these render the card in a live and +/// read the margins back, so they also prove the control applies the indent to the label and only the label. +/// They need STA and WPF. ShowRuntimeSummary stays private, called through reflection (the shape +/// PlanViewer4632FallbackTests in Lite.Tests uses for CriticalOrangeBrush), so this adds no +/// product surface; is not used because it awaits Task.Run +/// and nothing here pumps a dispatcher. +/// +public sealed class PlanViewerRuntimeSummaryNestingTests +{ + private static PlanStatement StatementWith(string? optimizationLevel, string? earlyAbortReason) => new() + { + StatementText = "SELECT 1", + StatementOptmLevel = optimizationLevel, + StatementOptmEarlyAbortReason = earlyAbortReason, + CardinalityEstimationModelVersion = 160 + }; + + [Fact] + public void RuntimeSummary_EarlyAbortUnderOptimization_IndentsItsLabelOnly() + { + OnStaThread(() => + { + var control = new PlanViewerControl(); + try + { + var grid = RenderRuntimeSummary(control, StatementWith("FULL", "TimeOut")); + + var ceModel = RowOf(grid, "CE model"); + var optimization = RowOf(grid, "Optimization"); + var earlyAbort = RowOf(grid, "Early abort"); + + // Optimization, then its reason on the next row, at the end of the card. + Assert.Equal(Grid.GetRow(ceModel.Label) + 1, Grid.GetRow(optimization.Label)); + Assert.Equal(Grid.GetRow(optimization.Label) + 1, Grid.GetRow(earlyAbort.Label)); + Assert.Equal(grid.RowDefinitions.Count - 1, Grid.GetRow(earlyAbort.Label)); + + // The label alone moves: 12px in, every other margin edge as on the un-nested rows. + Assert.Equal(new Thickness(12, 1, 8, 1), earlyAbort.Label.Margin); + Assert.Equal(new Thickness(0, 1, 8, 1), optimization.Label.Margin); + Assert.Equal(new Thickness(0, 1, 8, 1), ceModel.Label.Margin); + + // The value stays where every other value sits: same column, same margin. + Assert.Equal("TimeOut", earlyAbort.Value.Text); + Assert.Equal(1, Grid.GetColumn(earlyAbort.Value)); + Assert.Equal(Grid.GetColumn(optimization.Value), Grid.GetColumn(earlyAbort.Value)); + Assert.Equal(optimization.Value.Margin, earlyAbort.Value.Margin); + + // Nothing but the early abort label is indented. + Assert.All( + grid.Children.OfType().Where(t => Grid.GetColumn(t) == 0 && t.Text != "Early abort"), + t => Assert.Equal(0d, t.Margin.Left)); + } + finally + { + control.Cleanup(); + } + }); + } + + [Fact] + public void RuntimeSummary_EarlyAbortWithNoOptimizationRow_IsNotIndented() + { + OnStaThread(() => + { + var control = new PlanViewerControl(); + try + { + var grid = RenderRuntimeSummary(control, StatementWith(optimizationLevel: null, earlyAbortReason: "TimeOut")); + + var earlyAbort = RowOf(grid, "Early abort"); + + Assert.Equal(new Thickness(0, 1, 8, 1), earlyAbort.Label.Margin); + Assert.Equal(1, Grid.GetColumn(earlyAbort.Value)); + } + finally + { + control.Cleanup(); + } + }); + } + + /* ── card lookups ── */ + + /// Runs the control's private ShowRuntimeSummary and returns the card's grid. + private static Grid RenderRuntimeSummary(PlanViewerControl control, PlanStatement statement) + { + var method = typeof(PlanViewerControl).GetMethod("ShowRuntimeSummary", BindingFlags.NonPublic | BindingFlags.Instance); + Assert.True(method is not null, "PlanViewerControl.ShowRuntimeSummary no longer exists under that name - this pin's reflection anchor moved."); + method!.Invoke(control, new object[] { statement }); + + return Assert.IsType(Assert.Single(control.RuntimeSummaryContent.Children)); + } + + /// The label (column 0) and value (column 1) TextBlocks of the card row with this label. + private static (TextBlock Label, TextBlock Value) RowOf(Grid grid, string label) + { + var texts = grid.Children.OfType().ToList(); + var labelText = texts.Single(t => Grid.GetColumn(t) == 0 && t.Text == label); + var valueText = texts.Single(t => Grid.GetColumn(t) == 1 && Grid.GetRow(t) == Grid.GetRow(labelText)); + return (labelText, valueText); + } + + /// WPF objects require STA; same shape as the other WPF tests here. + private static void OnStaThread(Action body) + { + Exception? error = null; + var thread = new Thread(() => + { + try { body(); } + catch (Exception ex) { error = ex; } + }); + thread.SetApartmentState(ApartmentState.STA); + thread.Start(); + thread.Join(); + + if (error is not null) + { + throw error; + } + } +} diff --git a/Darling/Darling.Tests/Viewer4570Tests.cs b/Darling/Darling.Tests/Viewer4570Tests.cs index 00b2a61db..ff726f854 100644 --- a/Darling/Darling.Tests/Viewer4570Tests.cs +++ b/Darling/Darling.Tests/Viewer4570Tests.cs @@ -132,7 +132,7 @@ public void MemoryGrantColorKey_LowUtilizationNoSpill_IsError() // --- BuildRuntimeSummaryRows: row order (E11) ------------------------------------------ [Fact] - public void BuildRuntimeSummaryRows_ActualPlan_OrdersElapsedCpuElapsedDopCpuCompileMemoryOptCe() + public void BuildRuntimeSummaryRows_ActualPlan_OrdersElapsedCpuElapsedDopCpuCompileMemoryCeOptEarlyAbort() { var stmt = Statement(); stmt.QueryTimeStats = new QueryTimeInfo { ElapsedTimeMs = 1000, CpuTimeMs = 2000 }; @@ -140,15 +140,52 @@ public void BuildRuntimeSummaryRows_ActualPlan_OrdersElapsedCpuElapsedDopCpuComp stmt.CompileTimeMs = 5; stmt.MemoryGrant = new MemoryGrantInfo { GrantedMemoryKB = 1024, MaxUsedMemoryKB = 512 }; stmt.StatementOptmLevel = "FULL"; + stmt.StatementOptmEarlyAbortReason = "TimeOut"; stmt.CardinalityEstimationModelVersion = 160; var labels = PlanDisplayText.BuildRuntimeSummaryRows(stmt).Select(r => r.Label).ToArray(); + // #4836 (PerformanceStudio#613/#614): CE model moved above Optimization, and Early abort + // follows Optimization, so Optimization and its reason end the card. Assert.Equal( - new[] { "Elapsed", "CPU:Elapsed", "DOP", "CPU", "Compile", "Memory grant", "Optimization", "CE model" }, + new[] { "Elapsed", "CPU:Elapsed", "DOP", "CPU", "Compile", "Memory grant", "CE model", "Optimization", "Early abort" }, labels); } + // --- #4836: the early abort reason nests under Optimization ---------------------------- + + [Fact] + public void BuildRuntimeSummaryRows_EarlyAbortWithOptimizationLevel_IsNestedAndNothingElseIs() + { + var stmt = Statement(); + stmt.QueryTimeStats = new QueryTimeInfo { ElapsedTimeMs = 1000, CpuTimeMs = 2000 }; + stmt.StatementOptmLevel = "FULL"; + stmt.StatementOptmEarlyAbortReason = "GoodEnoughPlanFound"; + stmt.CardinalityEstimationModelVersion = 160; + + var rows = PlanDisplayText.BuildRuntimeSummaryRows(stmt); + + var earlyAbort = rows.Single(r => r.Label == "Early abort"); + Assert.Equal("GoodEnoughPlanFound", earlyAbort.Value); + Assert.True(earlyAbort.Nested); + Assert.False(rows.Single(r => r.Label == "Optimization").Nested); + Assert.All(rows.Where(r => r.Label != "Early abort"), r => Assert.False(r.Nested, r.Label)); + } + + [Fact] + public void BuildRuntimeSummaryRows_EarlyAbortWithoutOptimizationLevel_IsNotNested() + { + var stmt = Statement(); + stmt.StatementOptmEarlyAbortReason = "TimeOut"; + stmt.CardinalityEstimationModelVersion = 160; + + var rows = PlanDisplayText.BuildRuntimeSummaryRows(stmt); + + // No Optimization row to sit under, so the reason is a plain row, still after CE model. + Assert.Equal(new[] { "CE model", "Early abort" }, rows.Select(r => r.Label).ToArray()); + Assert.All(rows, r => Assert.False(r.Nested, r.Label)); + } + [Fact] public void BuildRuntimeSummaryRows_EstimatedPlan_SkipsRuntimeOnlyRows() { diff --git a/PerformanceMonitor.PlanAnalysis/PlanDisplayText.cs b/PerformanceMonitor.PlanAnalysis/PlanDisplayText.cs index 502b52fdf..83fcfb9f6 100644 --- a/PerformanceMonitor.PlanAnalysis/PlanDisplayText.cs +++ b/PerformanceMonitor.PlanAnalysis/PlanDisplayText.cs @@ -7,8 +7,10 @@ namespace PerformanceMonitor.PlanAnalysis; /// /// One row of the runtime summary card: a label, a display value, and an optional theme brush /// resource key ("ErrorBrush"/"WarningBrush"), where null means the card's default value color. +/// Nested marks a row that is a detail of the row above it (the early abort reason under +/// Optimization): the card indents its label, and its value stays in the same column as every other. /// -public readonly record struct RuntimeSummaryRow(string Label, string Value, string? ColorKey = null); +public readonly record struct RuntimeSummaryRow(string Label, string Value, string? ColorKey = null, bool Nested = false); /// /// Small, pure display-text helpers shared by every plan-analysis surface (viewer, MCP, drill-down), @@ -143,9 +145,12 @@ public static string FormatMemoryGrantKB(long kb) /// /// Builds the runtime summary card's rows in erikdarlingdata/PerformanceStudio@40ade29 (E11)'s /// order: Elapsed, CPU:Elapsed, DOP (or Serial reason), CPU, UDF CPU, UDF elapsed, Compile, - /// Cached plan size, Memory grant, Branches, Threads, Optimization, Early abort, CE model. A row - /// is omitted entirely when its underlying value isn't present, matching the WPF card's own - /// omission rules. + /// Cached plan size, Memory grant, Branches, Threads, then, as erikdarlingdata/PerformanceStudio#613 + /// and #614 reordered them, CE model, Optimization, Early abort. The early abort reason is part + /// of the optimization result rather than a fact of its own, so its row sits under Optimization + /// and is when the Optimization row is present; with no + /// optimization level there is nothing to nest under and it is a plain row. A row is omitted + /// entirely when its underlying value isn't present, matching the WPF card's own omission rules. /// public static IReadOnlyList BuildRuntimeSummaryRows( PlanStatement statement) @@ -248,12 +253,13 @@ public static IReadOnlyList BuildRuntimeSummaryRows( } } - if (!string.IsNullOrEmpty(statement.StatementOptmLevel)) - rows.Add(new RuntimeSummaryRow("Optimization", statement.StatementOptmLevel)); - if (!string.IsNullOrEmpty(statement.StatementOptmEarlyAbortReason)) - rows.Add(new RuntimeSummaryRow("Early abort", statement.StatementOptmEarlyAbortReason)); if (statement.CardinalityEstimationModelVersion > 0) rows.Add(new RuntimeSummaryRow("CE model", statement.CardinalityEstimationModelVersion.ToString())); + var hasOptimizationRow = !string.IsNullOrEmpty(statement.StatementOptmLevel); + if (hasOptimizationRow) + rows.Add(new RuntimeSummaryRow("Optimization", statement.StatementOptmLevel!)); + if (!string.IsNullOrEmpty(statement.StatementOptmEarlyAbortReason)) + rows.Add(new RuntimeSummaryRow("Early abort", statement.StatementOptmEarlyAbortReason, Nested: hasOptimizationRow)); return rows; } diff --git a/PerformanceMonitor.Ui/PlanViewerControl.Properties.cs b/PerformanceMonitor.Ui/PlanViewerControl.Properties.cs index 6b2dbac08..a377cc920 100644 --- a/PerformanceMonitor.Ui/PlanViewerControl.Properties.cs +++ b/PerformanceMonitor.Ui/PlanViewerControl.Properties.cs @@ -1610,7 +1610,10 @@ private void ShowRuntimeSummary(PlanStatement statement) grid.ColumnDefinitions.Add(new ColumnDefinition { Width = new GridLength(1, GridUnitType.Star) }); int rowIndex = 0; - void AddRow(string label, string value, string? colorKey) + // nested (#4836): the row is a detail of the row above it (the early abort reason under + // Optimization), so only its label gets a 12px left indent; its value stays in the same + // column as every other value, as in erikdarlingdata/PerformanceStudio#614. + void AddRow(string label, string value, string? colorKey, bool nested) { grid.RowDefinitions.Add(new RowDefinition { Height = GridLength.Auto }); @@ -1620,7 +1623,7 @@ void AddRow(string label, string value, string? colorKey) FontSize = 11, Foreground = labelBrush, HorizontalAlignment = HorizontalAlignment.Left, - Margin = new Thickness(0, 1, 8, 1) + Margin = new Thickness(nested ? 12 : 0, 1, 8, 1) }; Grid.SetRow(labelText, rowIndex); Grid.SetColumn(labelText, 0); @@ -1641,7 +1644,7 @@ void AddRow(string label, string value, string? colorKey) } foreach (var row in PlanDisplayText.BuildRuntimeSummaryRows(statement)) - AddRow(row.Label, row.Value, row.ColorKey); + AddRow(row.Label, row.Value, row.ColorKey, row.Nested); RuntimeSummaryContent.Children.Add(grid); SetInsightQuiet(RuntimeSummaryTitle, TooltipFgBrush, RuntimeSummaryAccent, isEmpty: false);