Plan analysis: operator self-time excludes the coordinator thread and looks through Compute Scalar and batch mode subtrees (#4513, #4515)

Darling/Darling.Tests/PlanSync4513Tests.cs

@@ -0,0 +1,169 @@
+/*
+ * 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.Collections.Generic;
+using PerformanceMonitor.PlanAnalysis;
+using Xunit;
+
+namespace Darling.Tests;
+
+/// <summary>
+/// #4513 — a parallel operator's own elapsed time (<see cref="PlanAnalyzer.GetOperatorOwnElapsedMs"/>, the
+/// per-thread path) had two faults that both inflate an operator until it outranks the operator that actually
+/// did the work.
+///
+/// <para>(a) Thread 0 is the coordinator in a parallel plan: it carries no rows, and its own
+/// <c>ActualElapsedMs</c> is the wall clock of the whole parallel branch, not a worker's own time. Including it
+/// handed an operator near the top of a parallel branch the branch's entire duration as its own work.</para>
+///
+/// <para>(b) The per-thread child subtraction only summed a direct child's own reported per-thread value. A
+/// batch-mode child reports STANDALONE time (not cumulative), so a grandchild inside the same batch zone kept
+/// its time inside the parent's self-time — while the grandchild also reported that same time as its own self
+/// time, so the time was counted twice across two rows of the same plan.</para>
+///
+/// <para>(c) A Compute Scalar child commonly carries no per-thread runtime stats at all (a pass-through), so the
+/// old code subtracted zero for it and the parent absorbed the whole subtree beneath it.</para>
+///
+/// <para>Synthetic <see cref="PlanNode"/> trees rather than full ShowPlan XML fixtures: the property under test
+/// is arithmetic over <see cref="PerThreadRuntimeInfo"/> rows, so each tree is built to have exactly the shape
+/// the assertion needs and the expected numbers are facts of the test, computed by hand below.</para>
+/// </summary>
+public sealed class PlanSync4513Tests
+{
+    private static PerThreadRuntimeInfo Thread(int id, long elapsedMs) =>
+        new() { ThreadId = id, ActualElapsedMs = elapsedMs };
+
+    /// <summary>(a) Thread 0 (the coordinator) is excluded from a parallel operator's self-time. With no
+    /// children, self-time is just the operator's own per-thread elapsed, maxed over WORKER threads only.
+    /// Coordinator elapsed (1000ms, the whole branch's wall clock) must not win the max.
+    /// Expected: max(50, 45) = 50ms. The unfixed code includes thread 0 and returns 1000ms.</summary>
+    [Fact]
+    public void CoordinatorThreadZero_IsExcludedFromSelfTime()
+    {
+        var node = new PlanNode
+        {
+            PhysicalOp = "Hash Match",
+            HasActualStats = true,
+            PerThreadStats = new List<PerThreadRuntimeInfo>
+            {
+                Thread(0, 1000),
+                Thread(1, 50),
+                Thread(2, 45),
+            },
+        };
+
+        var self = PlanAnalyzer.GetOperatorOwnElapsedMs(node);
+
+        Assert.Equal(50, self);
+    }
+
+    /// <summary>(b) A batch-mode child's subtree is summed, not just its own standalone value, so a grandchild
+    /// in the same batch zone doesn't stay behind inside the parent's self-time (which would double-count it
+    /// against the grandchild's own separately-reported self-time).
+    ///
+    /// Shape: Parent (row mode) -> Child (batch, standalone own time) -> Grandchild (batch, standalone own
+    /// time, no children). Per thread, Parent's cumulative elapsed = its own work + Child's own + Grandchild's
+    /// own:
+    ///   thread 1: 5 (parent's own) + 10 (child's own) + 20 (grandchild's own) = 35
+    ///   thread 2: 4 + 8 + 15 = 27
+    /// Correct self-time = max(35 - (10+20), 27 - (8+15)) = max(5, 4) = 5ms.
+    /// The unfixed code only subtracts Child's own value (10, 8), leaving Grandchild's time inside Parent's
+    /// self-time: max(35-10, 27-8) = max(25, 19) = 25ms.</summary>
+    [Fact]
+    public void BatchModeGrandchild_IsNotLeftInsideParentSelfTime()
+    {
+        var grandchild = new PlanNode
+        {
+            PhysicalOp = "Hash Match",
+            ActualExecutionMode = "Batch",
+            HasActualStats = true,
+            ActualElapsedMs = 20,
+            PerThreadStats = new List<PerThreadRuntimeInfo>
+            {
+                Thread(1, 20),
+                Thread(2, 15),
+            },
+        };
+        var child = new PlanNode
+        {
+            PhysicalOp = "Compute Scalar",
+            ActualExecutionMode = "Batch",
+            HasActualStats = true,
+            ActualElapsedMs = 10,
+            PerThreadStats = new List<PerThreadRuntimeInfo>
+            {
+                Thread(1, 10),
+                Thread(2, 8),
+            },
+            Children = new List<PlanNode> { grandchild },
+        };
+        var parent = new PlanNode
+        {
+            PhysicalOp = "Nested Loops",
+            HasActualStats = true,
+            ActualElapsedMs = 35,
+            PerThreadStats = new List<PerThreadRuntimeInfo>
+            {
+                Thread(1, 35),
+                Thread(2, 27),
+            },
+            Children = new List<PlanNode> { child },
+        };
+
+        var self = PlanAnalyzer.GetOperatorOwnElapsedMs(parent);
+
+        Assert.Equal(5, self);
+    }
+
+    /// <summary>(c) A Compute Scalar child with no per-thread runtime stats at all (a pass-through) is looked
+    /// through to its own child, so the real work beneath it still comes off the parent's self-time.
+    ///
+    /// Shape: Parent (row mode) -> Compute Scalar (no PerThreadStats) -> RealWork (row mode, has stats).
+    ///   thread 1: 5 (parent's own) + 40 (real work) = 45
+    ///   thread 2: 4 + 35 = 39
+    /// Correct self-time = max(45-40, 39-35) = max(5, 4) = 5ms.
+    /// The unfixed code finds no per-thread stats on the Compute Scalar child and subtracts nothing:
+    /// max(45, 39) = 45ms.</summary>
+    [Fact]
+    public void ComputeScalarPassThroughChild_IsLookedThrough()
+    {
+        var realWork = new PlanNode
+        {
+            PhysicalOp = "Clustered Index Scan",
+            HasActualStats = true,
+            ActualElapsedMs = 40,
+            PerThreadStats = new List<PerThreadRuntimeInfo>
+            {
+                Thread(1, 40),
+                Thread(2, 35),
+            },
+        };
+        var computeScalar = new PlanNode
+        {
+            PhysicalOp = "Compute Scalar",
+            HasActualStats = false,
+            PerThreadStats = new List<PerThreadRuntimeInfo>(),
+            Children = new List<PlanNode> { realWork },
+        };
+        var parent = new PlanNode
+        {
+            PhysicalOp = "Nested Loops",
+            HasActualStats = true,
+            PerThreadStats = new List<PerThreadRuntimeInfo>
+            {
+                Thread(1, 45),
+                Thread(2, 39),
+            },
+            Children = new List<PlanNode> { computeScalar },
+        };
+
+        var self = PlanAnalyzer.GetOperatorOwnElapsedMs(parent);
+
+        Assert.Equal(5, self);
+    }
+}

Darling/Darling.Tests/PlanSync4515Tests.cs

@@ -0,0 +1,109 @@
+/*
+ * 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.Collections.Generic;
+using PerformanceMonitor.PlanAnalysis;
+using Xunit;
+
+namespace Darling.Tests;
+
+/// <summary>
+/// #4515 — a SERIAL operator's own elapsed time (<see cref="PlanAnalyzer.GetOperatorOwnElapsedMs"/>, the
+/// serial row-mode path) only subtracted a direct child's own <c>ActualElapsedMs</c>. That misses the same two
+/// shapes #4513 fixed on the per-thread path, reusing the same two helpers there:
+///
+/// <para>(a) A Compute Scalar child commonly carries no runtime stats at all (<c>ActualElapsedMs</c> is 0), so
+/// the old code subtracted zero for it and the parent absorbed the whole subtree beneath it as its own
+/// self-time.</para>
+///
+/// <para>(b) A batch-mode child reports STANDALONE time (not cumulative), so a grandchild inside the same batch
+/// zone kept its time inside the parent's self-time — double-counted, since the grandchild also reports that
+/// same time as its own self-time on its own row of the plan.</para>
+///
+/// <para>Synthetic <see cref="PlanNode"/> trees rather than full ShowPlan XML fixtures, for the same reason as
+/// #4513's tests: the property under test is arithmetic, so each tree is built to have exactly the shape the
+/// assertion needs, with expected numbers worked out by hand below.</para>
+/// </summary>
+public sealed class PlanSync4515Tests
+{
+    /// <summary>(a) A Compute Scalar child with no runtime stats is looked through to its own child, so the
+    /// real work beneath it still comes off the parent's self-time.
+    ///
+    /// Shape: Parent (row mode, ActualElapsedMs=45) -> Compute Scalar (HasActualStats=false,
+    /// ActualElapsedMs=0) -> RealWork (row mode, ActualElapsedMs=40).
+    /// Correct self-time = 45 - 40 = 5ms.
+    /// The unfixed code subtracts the Compute Scalar's own ActualElapsedMs (0) and returns 45ms.</summary>
+    [Fact]
+    public void ComputeScalarPassThroughChild_IsLookedThrough()
+    {
+        var realWork = new PlanNode
+        {
+            PhysicalOp = "Clustered Index Scan",
+            HasActualStats = true,
+            ActualElapsedMs = 40,
+        };
+        var computeScalar = new PlanNode
+        {
+            PhysicalOp = "Compute Scalar",
+            HasActualStats = false,
+            ActualElapsedMs = 0,
+            Children = new List<PlanNode> { realWork },
+        };
+        var parent = new PlanNode
+        {
+            PhysicalOp = "Nested Loops",
+            HasActualStats = true,
+            ActualElapsedMs = 45,
+            Children = new List<PlanNode> { computeScalar },
+        };
+
+        var self = PlanAnalyzer.GetOperatorOwnElapsedMs(parent);
+
+        Assert.Equal(5, self);
+    }
+
+    /// <summary>(b) A batch-mode child's whole contiguous subtree is summed, not just its own standalone
+    /// value, so a grandchild in the same batch zone doesn't stay behind inside the parent's self-time.
+    ///
+    /// Shape: Parent (row mode, ActualElapsedMs=35) -> Child (batch, ActualElapsedMs=10, standalone own time)
+    /// -> Grandchild (batch, ActualElapsedMs=20, standalone own time, no children).
+    /// Correct effective child elapsed = 10 (child's own) + 20 (grandchild's own) = 30.
+    /// Correct self-time = 35 - 30 = 5ms.
+    /// The unfixed code only subtracts the direct child's own value (10), leaving the grandchild's time
+    /// inside the parent's self-time: 35 - 10 = 25ms.</summary>
+    [Fact]
+    public void BatchModeGrandchild_IsSummedIntoSubtree()
+    {
+        var grandchild = new PlanNode
+        {
+            PhysicalOp = "Hash Match",
+            ActualExecutionMode = "Batch",
+            HasActualStats = true,
+            ActualElapsedMs = 20,
+        };
+        var child = new PlanNode
+        {
+            PhysicalOp = "Compute Scalar",
+            ActualExecutionMode = "Batch",
+            HasActualStats = true,
+            ActualElapsedMs = 10,
+            Children = new List<PlanNode> { grandchild },
+        };
+        var parent = new PlanNode
+        {
+            PhysicalOp = "Nested Loops",
+            HasActualStats = true,
+            ActualElapsedMs = 35,
+            Children = new List<PlanNode> { child },
+        };
+
+        var self = PlanAnalyzer.GetOperatorOwnElapsedMs(parent);
+
+        Assert.Equal(5, self);
+    }
+}

Darling/Darling.Tests/PlanSync4515bTests.cs

@@ -0,0 +1,72 @@
+/*
+ * 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.Collections.Generic;
+using PerformanceMonitor.PlanAnalysis;
+using Xunit;
+
+namespace Darling.Tests;
+
+/// <summary>
+/// #4515 follow-up: <see cref="PlanAnalyzer.GetOperatorOwnElapsedMs"/>'s Parallelism (exchange) branch read
+/// <c>child.Children.Max(c => c.ActualElapsedMs)</c> — the exchange's own direct child's raw elapsed — instead
+/// of recursing through <c>GetEffectiveChildElapsedMs</c> the way every other branch does. That means a
+/// row-mode parent above an exchange above a batch-mode zone (or a no-stats Compute Scalar) only subtracted
+/// the exchange's direct child's OWN elapsed, not that child's effective (look-through) elapsed — the same
+/// under-subtraction #4515 fixed for the non-exchange case, just one hop further down.
+///
+/// <para>Shape: Parent (row mode, ActualElapsedMs=100) -> Parallelism exchange (Children.Count=1) ->
+/// BatchChild (batch mode, ActualElapsedMs=30, own time) -> BatchGrandchild (batch mode, ActualElapsedMs=50,
+/// own time, no children).</para>
+///
+/// <para>Correct effective elapsed for the exchange's child is the whole batch zone: 30 (BatchChild's own) +
+/// 50 (BatchGrandchild's own) = 80. Correct self-time = 100 - 80 = 20ms.</para>
+///
+/// <para>The unfixed code takes <c>Max(c => c.ActualElapsedMs)</c> over the exchange's direct children, i.e.
+/// just BatchChild's own 30, giving self-time = 100 - 30 = 70ms.</para>
+/// </summary>
+public sealed class PlanSync4515bTests
+{
+    [Fact]
+    public void ParallelismExchange_LooksThroughToChildsEffectiveElapsed()
+    {
+        var batchGrandchild = new PlanNode
+        {
+            PhysicalOp = "Hash Match",
+            ActualExecutionMode = "Batch",
+            HasActualStats = true,
+            ActualElapsedMs = 50,
+        };
+        var batchChild = new PlanNode
+        {
+            PhysicalOp = "Hash Match",
+            ActualExecutionMode = "Batch",
+            HasActualStats = true,
+            ActualElapsedMs = 30,
+            Children = new List<PlanNode> { batchGrandchild },
+        };
+        var exchange = new PlanNode
+        {
+            PhysicalOp = "Parallelism",
+            HasActualStats = true,
+            ActualElapsedMs = 80,
+            Children = new List<PlanNode> { batchChild },
+        };
+        var parent = new PlanNode
+        {
+            PhysicalOp = "Nested Loops",
+            HasActualStats = true,
+            ActualElapsedMs = 100,
+            Children = new List<PlanNode> { exchange },
+        };
+
+        var self = PlanAnalyzer.GetOperatorOwnElapsedMs(parent);
+
+        Assert.Equal(20, self);
+    }
+}