Skip to content

Expensive Operator (rule 35) can name an exchange operator when the operator next to it did the work #4690

Description

@erikdarlingdata

A plan analysis fix from PerformanceStudio (PS) that PerformanceMonitor (PM) still needs. It comes from PS PR #608, which merged after the PS-to-PM sync in #4511 was finished.

Summary

Rule 35 (PlanAnalyzer.cs:1554 to 1578) names an operator as expensive when its own elapsed time is at least 20% of the statement's elapsed time. Its condition (PlanAnalyzer.cs:1562 and 1563) has no exchange check. So the rule can name a Parallelism operator: Gather Streams, Distribute Streams or Repartition Streams. An exchange spends most of its time waiting on the operators that feed it and drain it. The warning then points at the exchange while the operator next to it is the real cost.

PM already treats exchange times as unreliable elsewhere. The comment on GetOperatorOwnElapsedMs says so (PlanAnalyzer.cs:2000 and 2001). The child-time code looks through an exchange to its dominant child (PlanAnalyzer.cs:2274 to 2276). Rule 35 does not apply that to the exchange node itself.

PM computes an operator's own time the way PS did before the fix: batch mode, then per thread, then serial (PlanAnalyzer.cs:2003 to 2014). Nothing on that path treats an exchange node differently. For example, a lone Parallelism node with 6,000 ms in a 10,000 ms statement gets the warning, because the serial path returns 6,000 ms.

Rule 35 only runs on a plan with actual times. It needs HasActualStats, which the parser sets only when an operator has RunTimeInformation (ShowPlanParser.cs:1494 to 1497). It also needs QueryTimeStats of at least 1,000 ms, which the parser sets only when the plan has that element (ShowPlanParser.cs:790 to 794). The code shows these PM sources of such plans:

  • Lite's Get Actual Plan runs the query with SET STATISTICS XML ON (ServerTab.Plans.cs:512).
  • The query snapshot collector stores a live_query_plan from sys.dm_exec_query_statistics_xml for a running query (QuerySnapshotsCollector.cs:406). Lite opens it before the cached plan (ServerTab.Plans.cs:349) and saves it as an actual plan (ServerTab.Plans.cs:209). The code does not show whether a plan captured while the query runs has QueryTimeStats. If it has none, Rule 35 stays silent on it.
  • An actual plan sent to the analyze_plan_xml MCP tool goes through the same pipeline (McpPlanAnalysisFormatter.cs:147).

PerformanceStudio

erikdarlingdata/PerformanceStudio@d59ac4c (erikdarlingdata/PerformanceStudio#608) makes Rule 35 skip exchanges. The condition gains && !NodeTimeAttribution.IsExchangeOperator(node) (PlanAnalyzer.Node.cs:993). The rule number, the warning type ("Expensive Operator"), the 20% share and the 1,000 ms floor do not change.

PS showed the problem on committed plans and on a live plan:

  • In serially-parallel.sqlplan, the Sort has 17,111 ms. The Repartition Streams below it, which feeds the Sort, has the same 17,111 ms. Both are at 100.0% of the statement.
  • In memory_grant_wait_plan.sqlplan and spill_plan.sqlplan, a Parallelism operator has 12,362 ms (46.1%).
  • On SQL Server 2025, a ROW_NUMBER() query over dbo.Posts joined to dbo.Users ran at DOP 8 in 41,119 ms with SET STATISTICS XML ON. Rule 35 named node 12, a Repartition Streams above the Posts scan and below a Sort that spilled: "Parallelism took 19,333ms (47.0% of statement elapsed)". Its slowest worker ran 21,051 ms, but the eight workers used only 19,398 ms of CPU together, about 2.4 seconds each. The statement had 312 seconds of CXSYNC_PORT wait and 210 seconds of CXPACKET wait across all threads. After the fix, node 12 has no Rule 35 warning and the Sort keeps its spill warning.

Two other live plans did not have an exchange named, before or after the fix. One was an 8-way hash join with an aggregate. The other was a windowed sort over dbo.Comments. The problem needs the right plan shape.

Port

Add an exchange check to the Rule 35 condition (PlanAnalyzer.cs:1562 and 1563). PS uses NodeTimeAttribution.IsExchangeOperator, and PM has no such helper. The PS helper is true when PhysicalOp is "Parallelism", or when LogicalOp is "Gather Streams", "Distribute Streams" or "Repartition Streams". Add a private helper with that test to PlanAnalyzer. PM already tests PhysicalOp == "Parallelism" in other places (PlanAnalyzer.cs:1460, 2107, 2163 and 2275). Those stay as they are.

Copy the PS comment (PlanAnalyzer.Node.cs:987 to 991) into the comment above Rule 35 (PlanAnalyzer.cs:1554 to 1561). The next reader must know why exchanges are skipped.

Leave GetOperatorOwnElapsedMs alone. The spill severity code (PlanAnalyzer.cs:958) and Rule 34 (PlanAnalyzer.cs:1179) also call it. The rule number, the warning type and the message do not change. The fix only removes warnings from exchange operators. The Sort in serially-parallel.sqlplan keeps its warning (PS test SeriallyParallelPlan_StillNamesTheSort).

PM has no counterpart of the PS file WarningBaseline.txt, and it does not have the three PS plan files. Port the tests that PS builds in code. They are in ExpensiveOperatorExchangeTests (tests/PlanViewer.Core.Tests). A node with 6,000 ms in a 10,000 ms statement is flagged when it is a Sort. It is not flagged when it is a Parallelism node with the logical operator Repartition Streams, Gather Streams or Distribute Streams.

PM's Rule 35 tests are in Darling/Darling.Tests/PlanSync4527Tests.cs. Darling/Darling.Tests/PlanSync4535NodeRulesATests.cs builds PlanNode trees in code, which is the style these tests need. Size S.

At 4c6dfe10. dev has no later change to PerformanceMonitor.PlanAnalysis.

No activity

Activity on this issue will appear here.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions