From fd7f22b4b134be3a675a45011d83c85d1ea1877a Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Mon, 9 Mar 2026 11:41:52 -0400 Subject: [PATCH] Fix exchange operator elapsed time and add Gather/Distribute/Repartition labels MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fix incorrect own-time calculation for exchange operators: Thread 0 (coordinator) elapsed time represents the entire parallel branch wall clock, not the operator's own work. Now returns 0 for Thread-0-only exchanges and uses max worker thread time when available. - Display LogicalOp on Parallelism nodes: "Parallelism (Gather Streams)", "Parallelism (Distribute Streams)", "Parallelism (Repartition Streams)" - Match CPU time font size to elapsed time (10 → 10, was 9) - Add "Copy Path" to tab right-click menu for file-backed plans Co-Authored-By: Claude Opus 4.6 --- .../Controls/PlanViewerControl.axaml.cs | 46 +++++++++++++++++-- src/PlanViewer.App/MainWindow.axaml.cs | 30 ++++++++++++ src/PlanViewer.Core/Output/TextFormatter.cs | 1 + src/PlanViewer.Core/Services/PlanAnalyzer.cs | 1 + .../Services/ShowPlanParser.cs | 1 + 5 files changed, 74 insertions(+), 5 deletions(-) diff --git a/src/PlanViewer.App/Controls/PlanViewerControl.axaml.cs b/src/PlanViewer.App/Controls/PlanViewerControl.axaml.cs index 82daa20..5bbc4fa 100644 --- a/src/PlanViewer.App/Controls/PlanViewerControl.axaml.cs +++ b/src/PlanViewer.App/Controls/PlanViewerControl.axaml.cs @@ -64,6 +64,11 @@ public partial class PlanViewerControl : UserControl private const double MaxZoom = 3.0; private string _label = ""; + /// + /// Full path on disk when the plan was loaded from a file. + /// + public string? SourceFilePath { get; set; } + // Node selection private Border? _selectedNodeBorder; private IBrush? _selectedNodeOriginalBorder; @@ -418,14 +423,21 @@ private Border CreateNodeVisual(PlanNode node, int totalWarningCount = -1) // Operator name var fgBrush = FindBrushResource("ForegroundBrush"); + // Operator name — for exchanges, show "Parallelism" + "(Gather Streams)" etc. + var opLabel = node.PhysicalOp; + if (node.PhysicalOp == "Parallelism" && !string.IsNullOrEmpty(node.LogicalOp) + && node.LogicalOp != "Parallelism") + { + opLabel = $"Parallelism\n({node.LogicalOp})"; + } stack.Children.Add(new TextBlock { - Text = node.PhysicalOp, + Text = opLabel, FontSize = 10, FontWeight = FontWeight.SemiBold, Foreground = fgBrush, TextAlignment = TextAlignment.Center, - TextTrimming = TextTrimming.CharacterEllipsis, + TextWrapping = TextWrapping.Wrap, MaxWidth = PlanLayoutEngine.NodeWidth - 16, HorizontalAlignment = HorizontalAlignment.Center }); @@ -471,7 +483,7 @@ private Border CreateNodeVisual(PlanNode node, int totalWarningCount = -1) stack.Children.Add(new TextBlock { Text = $"CPU: {ownCpuSec:F3}s", - FontSize = 9, + FontSize = 10, Foreground = cpuBrush, TextAlignment = TextAlignment.Center, HorizontalAlignment = HorizontalAlignment.Center @@ -2248,10 +2260,34 @@ private static long GetOwnElapsedMs(PlanNode node) if (node.ActualElapsedMs <= 0) return 0; var mode = node.ActualExecutionMode ?? node.ExecutionMode; if (mode == "Batch") return node.ActualElapsedMs; - var childSum = GetChildElapsedMsSum(node); - return Math.Max(0, node.ActualElapsedMs - childSum); + + // Exchange operators: Thread 0 is the coordinator whose elapsed time is the + // wall clock for the entire parallel branch — not the operator's own work. + if (IsExchangeOperator(node)) + { + // If we have worker thread data, use max of worker threads + var workerMax = node.PerThreadStats + .Where(t => t.ThreadId > 0) + .Select(t => t.ActualElapsedMs) + .DefaultIfEmpty(0) + .Max(); + if (workerMax > 0) + { + var childSum = GetChildElapsedMsSum(node); + return Math.Max(0, workerMax - childSum); + } + // Thread 0 only (coordinator) — exchange does negligible own work + return 0; + } + + var childElapsedSum = GetChildElapsedMsSum(node); + return Math.Max(0, node.ActualElapsedMs - childElapsedSum); } + private static bool IsExchangeOperator(PlanNode node) => + node.PhysicalOp == "Parallelism" + || node.LogicalOp is "Gather Streams" or "Distribute Streams" or "Repartition Streams"; + private static long GetChildCpuMsSum(PlanNode node) { long sum = 0; diff --git a/src/PlanViewer.App/MainWindow.axaml.cs b/src/PlanViewer.App/MainWindow.axaml.cs index 176fd9b..bc9d8a1 100644 --- a/src/PlanViewer.App/MainWindow.axaml.cs +++ b/src/PlanViewer.App/MainWindow.axaml.cs @@ -351,6 +351,7 @@ private void LoadPlanFile(string filePath) var viewer = new PlanViewerControl(); viewer.LoadPlan(xml, fileName); + viewer.SourceFilePath = filePath; // Wrap viewer with advice toolbar var content = CreatePlanTabContent(viewer); @@ -859,11 +860,17 @@ private TabItem CreateTab(string label, Control content) closeBtn.Click += CloseTab_Click; // Right-click context menu + var copyPathItem = new MenuItem { Header = "Copy Path", Tag = tab }; + // Only visible when tab content has a file path + var filePath = GetTabFilePath(tab); + copyPathItem.IsVisible = filePath != null; + var contextMenu = new ContextMenu { Items = { new MenuItem { Header = "Rename Tab", Tag = new object[] { header, headerText } }, + copyPathItem, new Separator(), new MenuItem { Header = "Close", Tag = tab, InputGesture = new KeyGesture(Key.W, KeyModifiers.Control) }, new MenuItem { Header = "Close Other Tabs", Tag = tab }, @@ -901,6 +908,15 @@ private void TabContextMenu_Click(object? sender, RoutedEventArgs e) StartRename((StackPanel)parts[0], (TextBlock)parts[1]); break; + case "Copy Path": + if (item.Tag is TabItem pathTab) + { + var path = GetTabFilePath(pathTab); + if (path != null) + _ = this.Clipboard?.SetTextAsync(path); + } + break; + case "Close": if (item.Tag is TabItem tab) { @@ -927,6 +943,20 @@ private void TabContextMenu_Click(object? sender, RoutedEventArgs e) } } + private static string? GetTabFilePath(TabItem tab) + { + // Plans opened from file are wrapped in a DockPanel with the viewer as the last child + if (tab.Content is DockPanel dp) + { + foreach (var child in dp.Children) + { + if (child is PlanViewerControl v) + return v.SourceFilePath; + } + } + return null; + } + private void StartRename(StackPanel header, TextBlock headerText) { var textBox = new TextBox diff --git a/src/PlanViewer.Core/Output/TextFormatter.cs b/src/PlanViewer.Core/Output/TextFormatter.cs index c11f02f..80001d1 100644 --- a/src/PlanViewer.Core/Output/TextFormatter.cs +++ b/src/PlanViewer.Core/Output/TextFormatter.cs @@ -430,4 +430,5 @@ private static long GetChildElapsedSum(OperatorResult node) } return sum; } + } diff --git a/src/PlanViewer.Core/Services/PlanAnalyzer.cs b/src/PlanViewer.Core/Services/PlanAnalyzer.cs index 18f7e00..26ad250 100644 --- a/src/PlanViewer.Core/Services/PlanAnalyzer.cs +++ b/src/PlanViewer.Core/Services/PlanAnalyzer.cs @@ -1003,6 +1003,7 @@ private static bool IsScanOperator(PlanNode node) !node.PhysicalOp.Contains("Constant", StringComparison.OrdinalIgnoreCase); } + /// /// Detects non-SARGable patterns in scan predicates. /// Returns a description of the issue, or null if the predicate is fine. diff --git a/src/PlanViewer.Core/Services/ShowPlanParser.cs b/src/PlanViewer.Core/Services/ShowPlanParser.cs index 86e2e74..baf847b 100644 --- a/src/PlanViewer.Core/Services/ShowPlanParser.cs +++ b/src/PlanViewer.Core/Services/ShowPlanParser.cs @@ -644,6 +644,7 @@ private static PlanNode ParseRelOp(XElement relOpEl) node.PhysicalOp = "Lazy " + node.PhysicalOp; } + // Map to icon node.IconName = PlanIconMapper.GetIconName(node.PhysicalOp);