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

[erikdarlingdata]

Fixes #4513
Fixes #4515
Part of #4511

Why

GetOperatorOwnElapsedMs's per-thread path (for parallel plans with per-thread runtime stats) and its serial path (for plans with a single set of runtime stats) shared two faults that both inflate an operator's reported self-time until it outranks the operator that actually did the work:

1. On the per-thread path, it iterated every thread, including thread 0. In a parallel plan thread 0 is the coordinator: it carries no rows, and its own ActualElapsedMs is the wall clock of the whole parallel branch, not a worker's own time. An operator near the top of a parallel branch could be handed the branch's entire duration as its own work.
2. On both paths, the child subtraction only summed a direct child's own reported value, looking through only a Parallelism (exchange) child. 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 also reporting that same time as its own self-time — double-counted across two rows of the plan. A Compute Scalar child commonly carries no runtime stats at all, so the old code subtracted zero for it and the parent absorbed the whole subtree beneath it.

Self-time feeds the "Operator time" text on findings, the benefit scores of the operator-time rules, and severity, so the wrong operator can look like the most expensive one in a plan.

What changes

PerformanceMonitor.PlanAnalysis/PlanAnalyzer.cs:

• WorkThreads(node): returns the node's per-thread stats with thread 0 excluded, falling back to all threads if that leaves none (a serial plan has a single thread numbered 0, which IS a worker).
• GetPerThreadOwnElapsed now builds its parent-side lookup from WorkThreads instead of every thread, and delegates the child side to AddEffectiveChildElapsedByThread.
• AddEffectiveChildElapsedByThread(child, acc): the per-thread mirror of the serial path's child look-through. It looks through a Parallelism (exchange) child to its dominant child, sums a batch-mode child's whole contiguous subtree via AddBatchSubtreeElapsedByThread, and looks through a child with no runtime stats (a Compute Scalar pass-through) to its own children.
• AddBatchSubtreeElapsedByThread(node, acc): sums a contiguous batch-mode zone per thread, stopping at exchange boundaries, since batch operators pipeline and their times add rather than nest.
• GetSerialOwnElapsed (the single-thread path) now delegates its own child subtraction to a new GetEffectiveChildElapsedMs(child), the non-per-thread twin of the two helpers above: it looks through a Parallelism child to its dominant child's ActualElapsedMs, sums a batch-mode child's whole contiguous subtree via a new SumBatchSubtreeElapsedMs, and looks through a no-stats pass-through child to its own children.
• SumBatchSubtreeElapsedMs(node): the non-per-thread twin of AddBatchSubtreeElapsedByThread — sums ActualElapsedMs across a contiguous batch-mode zone, stopping at exchange boundaries.
• GetEffectiveChildElapsedMs's Parallelism (exchange) branch now recurses (child.Children.Max(GetEffectiveChildElapsedMs)) instead of taking the exchange's direct child's own raw ActualElapsedMs. That closes a one-hop gap: a row-mode parent above an exchange above a batch-mode zone (or a no-stats Compute Scalar) was only subtracting the exchange child's own value, not that child's effective (look-through) value — the same under-subtraction fixed elsewhere in this change, one level further down through an exchange.

This mirrors PerformanceStudio's 0b689880e (parallel/per-thread path, PlanAnalyzer.Timing.cs) and 3a40ada/3fd40d1 (serial path, same file), adapted to PM's single PlanAnalyzer.cs file (PS splits timing helpers into a partial PlanAnalyzer.Timing.cs) and with the CPU-side twin of the helper left out — PM's GetOperatorOwnElapsedMs only has an elapsed-time path today, so there is no CPU-ms counterpart to port here.

Test plan

New Darling.Tests.PlanSync4513Tests (3 tests, per-thread path) and Darling.Tests.PlanSync4515Tests (2 tests, serial path), each a synthetic PlanNode/PerThreadRuntimeInfo tree built by hand rather than full ShowPlan XML, since the property under test is arithmetic:

• PlanSync4513Tests.CoordinatorThreadZero_IsExcludedFromSelfTime: an operator with per-thread elapsed {0: 1000, 1: 50, 2: 45} and no children. Expected self-time 50ms (max of worker threads). RED on dev: actual 1000ms.

• PlanSync4513Tests.BatchModeGrandchild_IsNotLeftInsideParentSelfTime: row-mode parent above a batch Compute Scalar above a batch Hash Match, with per-thread elapsed chosen so parent cumulative = own + child's own + grandchild's own. Expected self-time 5ms. RED on dev: actual 25ms (grandchild's time left inside the parent).

• PlanSync4513Tests.ComputeScalarPassThroughChild_IsLookedThrough: row-mode parent above a Compute Scalar with no per-thread stats above a real scan. Expected self-time 5ms. RED on dev: actual 45ms (nothing subtracted for the pass-through).

• PlanSync4515Tests.ComputeScalarPassThroughChild_IsLookedThrough: serial parent (ActualElapsedMs=45) above a Compute Scalar (HasActualStats=false, ActualElapsedMs=0) above real work (ActualElapsedMs=40). Expected self-time 5ms. RED on dev: actual 45ms.

• PlanSync4515Tests.BatchModeGrandchild_IsSummedIntoSubtree: serial parent (ActualElapsedMs=35) above a batch Compute Scalar (ActualElapsedMs=10) above a batch Hash Match grandchild (ActualElapsedMs=20, no children). Expected self-time 5ms (35 − (10+20)). RED on dev: actual 25ms (35 − 10, grandchild's time left inside the parent).

• PlanSync4515bTests.ParallelismExchange_LooksThroughToChildsEffectiveElapsed: serial parent (ActualElapsedMs=100) above a Parallelism exchange (ActualElapsedMs=80, one child) above a batch Hash Match child (ActualElapsedMs=30) above a batch Hash Match grandchild (ActualElapsedMs=50, no children). Correct effective elapsed for the exchange's child is the whole batch zone, 30+50=80, so self-time = 100 − 80 = 20ms. RED on the branch before the exchange branch recursed: Expected: 20 / Actual: 70.

• Mutations, each reverted: removing the thread-0 skip fails CoordinatorThreadZero_IsExcludedFromSelfTime (1000 vs 50); replacing the Compute Scalar look-through with return 0 fails ComputeScalarPassThroughChild_IsLookedThrough (45 vs 5); the exchange branch back on the child's own time fails ParallelismExchange_LooksThroughToChildsEffectiveElapsed (70 vs 20).

• Run: the three new classes plus the plan-analysis classes: Total: 237, Failed: 0 (the live-Postgres plan-tool class runs in CI; it also passed live on this branch before the last one-line change, 236 run). Lite.Tests builds.

• Mirrors erikdarlingdata/PerformanceStudio@0b68988 (Parallel operator self-time counts the coordinator thread and double-counts batch mode and Compute Scalar children #4513) and erikdarlingdata/PerformanceStudio@3a40ada + erikdarlingdata/PerformanceStudio@3fd40d1 (Serial operator self-time does not look through Compute Scalar or sum a batch mode subtree #4515). GetEffectiveChildElapsedMs matches PerformanceStudio's current version, including the look-through of an exchange to its child's effective time.

Note: self-time also feeds BenefitScorer, which no entry point calls yet (#4546), so benefit values change only once it does.

CHANGELOG

SECTION: Fixed
ENTRY: - Plan analysis: an operator's reported self-time no longer includes the coordinator thread's wall-clock time or double-counts a batch-mode or Compute Scalar child's time, in either a parallel or a serial plan ([#4547]) - the "Operator time" text and the severity of plan findings now reflect the operator that actually did the work, not an operator that happened to sit above a batch subtree, a Compute Scalar pass-through, or near the top of a parallel branch.
REF: [#4547]: #4547