diff --git a/src/PlanViewer.Core/Output/HtmlExporter.cs b/src/PlanViewer.Core/Output/HtmlExporter.cs index a2fe5c0..e945151 100644 --- a/src/PlanViewer.Core/Output/HtmlExporter.cs +++ b/src/PlanViewer.Core/Output/HtmlExporter.cs @@ -300,6 +300,11 @@ private static void WriteRuntimeCard(StringBuilder sb, StatementResult stmt) { WriteRow(sb, "Elapsed", $"{stmt.QueryTime.ElapsedTimeMs:N0} ms"); WriteRow(sb, "CPU", $"{stmt.QueryTime.CpuTimeMs:N0} ms"); + if (stmt.QueryTime.ElapsedTimeMs > 0) + { + var ratio = (double)stmt.QueryTime.CpuTimeMs / stmt.QueryTime.ElapsedTimeMs; + WriteRow(sb, "CPU:Elapsed", ratio.ToString("N2")); + } } if (stmt.DegreeOfParallelism > 0) WriteRow(sb, "DOP", stmt.DegreeOfParallelism.ToString()); diff --git a/src/PlanViewer.Core/Output/ResultMapper.cs b/src/PlanViewer.Core/Output/ResultMapper.cs index abfab98..fde71c7 100644 --- a/src/PlanViewer.Core/Output/ResultMapper.cs +++ b/src/PlanViewer.Core/Output/ResultMapper.cs @@ -271,7 +271,7 @@ private static OperatorResult MapNode(PlanNode node) Type = w.WarningType, Severity = w.Severity.ToString(), Message = w.Message, - Operator = $"{node.PhysicalOp} (Node {node.NodeId})", + Operator = FormatOperatorLabel(node), NodeId = node.NodeId, MaxBenefitPercent = w.MaxBenefitPercent, ActionableFix = w.ActionableFix @@ -316,4 +316,22 @@ private static void CollectNodeWarnings(OperatorResult? node, List + /// Formats an operator label for the Operator field on warnings. + /// Includes object name for data access operators (scans, seeks, lookups) + /// where it helps identify which table/index is involved. + /// + private static string FormatOperatorLabel(PlanNode node) + { + if (!string.IsNullOrEmpty(node.ObjectName)) + { + var objRef = !string.IsNullOrEmpty(node.DatabaseName) + ? $"{node.DatabaseName}.{node.ObjectName}" + : node.ObjectName; + return $"{node.PhysicalOp} on {objRef} (Node {node.NodeId})"; + } + + return $"{node.PhysicalOp} (Node {node.NodeId})"; + } } diff --git a/src/PlanViewer.Core/Services/BenefitScorer.cs b/src/PlanViewer.Core/Services/BenefitScorer.cs index 7fa24a0..b341b76 100644 --- a/src/PlanViewer.Core/Services/BenefitScorer.cs +++ b/src/PlanViewer.Core/Services/BenefitScorer.cs @@ -18,11 +18,11 @@ public static class BenefitScorer "Filter Operator", // Rule 1 "Eager Index Spool", // Rule 2 "Spill", // Rule 7 - "Key Lookup", // Rule 10 - "RID Lookup", // Rule 10 variant + // Key Lookup / RID Lookup (Rule 10) handled separately by ScoreKeyLookupWarning "Scan With Predicate", // Rule 11 "Non-SARGable Predicate", // Rule 12 "Scan Cardinality Misestimate", // Rule 32 + "Bare Scan", // Rule 34 }; public static void Score(ParsedPlan plan) @@ -50,44 +50,18 @@ private static void ScoreStatementWarnings(PlanStatement stmt) { switch (warning.WarningType) { - case "Ineffective Parallelism": // Rule 25 - case "Parallel Wait Bottleneck": // Rule 31 - // These are meta-findings about parallelism efficiency. - // The benefit is the gap between actual and ideal elapsed time. - if (elapsedMs > 0 && stmt.QueryTimeStats != null) - { - var cpu = stmt.QueryTimeStats.CpuTimeMs; - var dop = stmt.DegreeOfParallelism; - if (dop > 1 && cpu > 0) - { - // Ideal elapsed = CPU / DOP. Benefit = (actual - ideal) / actual - var idealElapsed = (double)cpu / dop; - var benefit = Math.Max(0, (elapsedMs - idealElapsed) / elapsedMs * 100); - warning.MaxBenefitPercent = Math.Min(100, Math.Round(benefit, 1)); - } - } - break; - case "Serial Plan": // Rule 3 - // Can't know how fast a parallel plan would be, but estimate: - // CPU-bound: benefit up to (1 - 1/maxDOP) * 100% - if (elapsedMs > 0 && stmt.QueryTimeStats != null) + // Per Joe's formula: (cpu * (DOP - 1) / DOP) / elapsed * 100 + // Assumes DOP 4 when the plan doesn't tell us. No benefit when cost < 1 + // (trivial plans don't gain from parallelism). + if (elapsedMs > 0 && stmt.QueryTimeStats != null && stmt.StatementSubTreeCost >= 1.0) { var cpu = stmt.QueryTimeStats.CpuTimeMs; - // Assume server max DOP — use a conservative 4 if unknown var potentialDop = 4; - if (cpu >= elapsedMs) - { - // CPU-bound: parallelism could help significantly - var benefit = (1.0 - 1.0 / potentialDop) * 100; - warning.MaxBenefitPercent = Math.Round(benefit, 1); - } - else + if (cpu > 0) { - // Not CPU-bound: parallelism helps less - var cpuRatio = (double)cpu / elapsedMs; - var benefit = cpuRatio * (1.0 - 1.0 / potentialDop) * 100; - warning.MaxBenefitPercent = Math.Round(Math.Min(50, benefit), 1); + var benefit = ((double)cpu * (potentialDop - 1) / potentialDop) / elapsedMs * 100; + warning.MaxBenefitPercent = Math.Round(Math.Min(100, benefit), 1); } } break; @@ -150,6 +124,10 @@ private static void ScoreNodeWarnings(PlanNode node, PlanStatement stmt) { ScoreSpillWarning(warning, node, stmt); } + else if (warning.WarningType is "Key Lookup" or "RID Lookup") // Rule 10 + { + ScoreKeyLookupWarning(warning, node, stmt); + } else if (OperatorTimeRules.Contains(warning.WarningType)) { ScoreByOperatorTime(warning, node, stmt); @@ -159,7 +137,8 @@ private static void ScoreNodeWarnings(PlanNode node, PlanStatement stmt) ScoreEstimateMismatchWarning(warning, node, stmt); } // Rules that stay null: Scalar UDF (Rule 6, informational reference), - // Parallel Skew (Rule 8), Data Type Mismatch (Rule 13), + // Parallel Skew (Rule 8 — will be integrated per-operator later), + // Data Type Mismatch (Rule 13), // Lazy Spool Ineffective (Rule 14), Join OR Clause (Rule 15), // Many-to-Many Merge Join (Rule 17), CTE Multiple References (Rule 21), // Table Variable (Rule 22), Table-Valued Function (Rule 23), @@ -282,6 +261,61 @@ private static void ScoreByOperatorTime(PlanWarning warning, PlanNode node, Plan } } + /// + /// Rule 10: Key Lookup / RID Lookup — benefit includes the lookup operator's time, + /// plus the parent Nested Loops join when the NL only exists to drive the lookup + /// (inner child is the lookup, outer child is a seek/scan with no subtree). + /// + private static void ScoreKeyLookupWarning(PlanWarning warning, PlanNode node, PlanStatement stmt) + { + var stmtMs = stmt.QueryTimeStats?.ElapsedTimeMs ?? 0; + + if (node.HasActualStats && stmtMs > 0) + { + var operatorMs = PlanAnalyzer.GetOperatorOwnElapsedMs(node); + + // Check if the parent NL join is purely a lookup driver: + // - Parent is Nested Loops + // - Has exactly 2 children + // - This node (the lookup) is the inner child (index 1) + // - The outer child (index 0) is a simple seek/scan with no children + var parent = node.Parent; + if (parent != null + && parent.PhysicalOp == "Nested Loops" + && parent.Children.Count == 2 + && parent.Children[1] == node + && parent.Children[0].Children.Count == 0) + { + operatorMs += PlanAnalyzer.GetOperatorOwnElapsedMs(parent); + } + + if (operatorMs > 0) + { + var benefit = (double)operatorMs / stmtMs * 100; + warning.MaxBenefitPercent = Math.Round(Math.Min(100, benefit), 1); + } + else + { + warning.MaxBenefitPercent = 0; + } + } + else if (!node.HasActualStats && stmt.StatementSubTreeCost > 0) + { + var benefit = (double)node.CostPercent; + // Same parent-NL logic for estimated plans + var parent = node.Parent; + if (parent != null + && parent.PhysicalOp == "Nested Loops" + && parent.Children.Count == 2 + && parent.Children[1] == node + && parent.Children[0].Children.Count == 0) + { + benefit += parent.CostPercent; + } + warning.MaxBenefitPercent = Math.Round(Math.Min(100, benefit), 1); + } + } + /// /// Rule 5: Row Estimate Mismatch — benefit is the harmed operator's time. /// If the mismatch caused a spill, benefit = spilling operator time. diff --git a/src/PlanViewer.Core/Services/PlanAnalyzer.cs b/src/PlanViewer.Core/Services/PlanAnalyzer.cs index 5e18d0a..01229ea 100644 --- a/src/PlanViewer.Core/Services/PlanAnalyzer.cs +++ b/src/PlanViewer.Core/Services/PlanAnalyzer.cs @@ -212,7 +212,7 @@ private static void AnalyzeStatement(PlanStatement stmt, AnalyzerConfig cfg) stmt.PlanWarnings.Add(new PlanWarning { WarningType = "Serial Plan", - Message = $"Query running serially: {reason}.", + Message = "Query running serially: MAXDOP is set to 1 using a query hint.", Severity = PlanWarningSeverity.Warning }); } @@ -252,10 +252,17 @@ private static void AnalyzeStatement(PlanStatement stmt, AnalyzerConfig cfg) { var grantMB = grant.GrantedMemoryKB / 1024.0; var usedMB = grant.MaxUsedMemoryKB / 1024.0; + var message = $"Granted {grantMB:N0} MB but only used {usedMB:N0} MB ({wasteRatio:F0}x overestimate). The unused memory is reserved and unavailable to other queries."; + + // Note adaptive joins that chose Nested Loops at runtime — the grant + // was sized for a hash join that never happened. + if (stmt.RootNode != null && HasAdaptiveJoinChoseNestedLoop(stmt.RootNode)) + message += " An adaptive join in this plan executed as a Nested Loop at runtime — the memory grant was sized for the hash join alternative that wasn't used."; + stmt.PlanWarnings.Add(new PlanWarning { WarningType = "Excessive Memory Grant", - Message = $"Granted {grantMB:N0} MB but only used {usedMB:N0} MB ({wasteRatio:F0}x overestimate). The unused memory is reserved and unavailable to other queries.", + Message = message, Severity = PlanWarningSeverity.Warning }); } @@ -272,16 +279,24 @@ private static void AnalyzeStatement(PlanStatement stmt, AnalyzerConfig cfg) }); } - // Large memory grant with sort/hash guidance + // Large memory grant with top consumers if (grant.GrantedMemoryKB >= 1048576 && stmt.RootNode != null) { var consumers = new List(); FindMemoryConsumers(stmt.RootNode, consumers); var grantMB = grant.GrantedMemoryKB / 1024.0; - var guidance = consumers.Count > 0 - ? $" Memory consumers: {string.Join(", ", consumers)}. Check whether these operators are processing more rows than necessary." - : ""; + var guidance = ""; + if (consumers.Count > 0) + { + // Show only the top 3 consumers — listing 20+ is noise + var shown = consumers.Take(3); + var remaining = consumers.Count - 3; + guidance = $" Largest consumers: {string.Join(", ", shown)}"; + if (remaining > 0) + guidance += $", and {remaining} more"; + guidance += "."; + } stmt.PlanWarnings.Add(new PlanWarning { @@ -370,53 +385,9 @@ private static void AnalyzeStatement(PlanStatement stmt, AnalyzerConfig cfg) }); } - // Rule 25: Ineffective parallelism — DOP-aware efficiency scoring - // Efficiency = (speedup - 1) / (DOP - 1) * 100 - // where speedup = CPU / Elapsed. At DOP 1 speedup=1 (0%), at DOP=speedup (100%). - // Rule 31: Parallel wait bottleneck — elapsed >> CPU means threads waiting, not working. - if (!cfg.IsRuleDisabled(25) && stmt.DegreeOfParallelism > 1 && stmt.QueryTimeStats != null) - { - var cpu = stmt.QueryTimeStats.CpuTimeMs; - var elapsed = stmt.QueryTimeStats.ElapsedTimeMs; - var dop = stmt.DegreeOfParallelism; - - if (elapsed >= 1000 && cpu > 0) - { - var speedup = (double)cpu / elapsed; - var efficiency = Math.Max(0.0, Math.Min(100.0, (speedup - 1.0) / (dop - 1.0) * 100.0)); - - // Build targeted advice from wait stats if available - var waitAdvice = GetWaitStatsAdvice(stmt.WaitStats); - - if (speedup < 0.5 && !cfg.IsRuleDisabled(31)) - { - // CPU well below Elapsed: threads are waiting, not doing CPU work - var waitPct = (1.0 - speedup) * 100; - var advice = waitAdvice ?? "Common causes include spills to tempdb, physical I/O reads, lock or latch contention, and memory grant waits."; - stmt.PlanWarnings.Add(new PlanWarning - { - WarningType = "Parallel Wait Bottleneck", - Message = $"Parallel plan (DOP {dop}, {efficiency:N0}% efficient) with elapsed time ({elapsed:N0}ms) exceeding CPU time ({cpu:N0}ms). " + - $"Approximately {waitPct:N0}% of elapsed time was spent waiting rather than on CPU. " + - advice, - Severity = PlanWarningSeverity.Warning - }); - } - else if (efficiency < 40) - { - // CPU >= Elapsed but well below DOP potential — parallelism is ineffective - var advice = waitAdvice ?? "Look for parallel thread skew, blocking exchanges, or serial zones in the plan that prevent effective parallel execution."; - stmt.PlanWarnings.Add(new PlanWarning - { - WarningType = "Ineffective Parallelism", - Message = $"Parallel plan (DOP {dop}) is only {efficiency:N0}% efficient — CPU time ({cpu:N0}ms) vs elapsed time ({elapsed:N0}ms). " + - $"At DOP {dop}, ideal CPU time would be ~{elapsed * dop:N0}ms. " + - advice, - Severity = efficiency < 20 ? PlanWarningSeverity.Critical : PlanWarningSeverity.Warning - }); - } - } - } + // Rules 25 (Ineffective Parallelism) and 31 (Parallel Wait Bottleneck) were removed. + // The CPU:Elapsed ratio is now shown in the runtime summary, and wait stats speak + // for themselves — no need for meta-warnings guessing at causes. // Rule 30: Missing index quality evaluation if (!cfg.IsRuleDisabled(30)) @@ -613,7 +584,8 @@ private static void AnalyzeNode(PlanNode node, PlanStatement stmt, AnalyzerConfi // - A parent join may have chosen the wrong strategy // - Root nodes with no parent to harm are skipped // - Nodes whose only parents are Parallelism/Top/Sort (no spill) are skipped - if (!cfg.IsRuleDisabled(5) && node.HasActualStats && node.EstimateRows > 0) + if (!cfg.IsRuleDisabled(5) && node.HasActualStats && node.EstimateRows > 0 + && !node.Lookup) // Key lookups are point lookups (1 row per execution) — per-execution estimate is misleading { if (node.ActualRows == 0) { @@ -659,7 +631,13 @@ private static void AnalyzeNode(PlanNode node, PlanStatement stmt, AnalyzerConfi } // Rule 6: Scalar UDF references (works on estimated plans too) - if (!cfg.IsRuleDisabled(6)) + // Suppress when Serial Plan warning is already firing for a UDF-related reason — + // the Serial Plan warning already explains the issue, this would be redundant. + var serialPlanCoversUdf = stmt.NonParallelPlanReason is + "TSQLUserDefinedFunctionsNotParallelizable" + or "CLRUserDefinedFunctionRequiresDataAccess" + or "CouldNotGenerateValidParallelPlan"; + if (!cfg.IsRuleDisabled(6) && !serialPlanCoversUdf) foreach (var udf in node.ScalarUdfs) { var type = udf.IsClrFunction ? "CLR" : "T-SQL"; @@ -752,6 +730,11 @@ private static void AnalyzeNode(PlanNode node, PlanStatement stmt, AnalyzerConfi message += " Batch mode sorts produce all output rows on a single thread by design, unless feeding a batch mode Window Aggregate."; severity = PlanWarningSeverity.Info; } + else + { + // Add practical context — skew is often hard to fix + message += " Common causes: uneven data distribution across partitions or hash buckets, or a scan/seek whose predicate sends most rows to one range. Reducing DOP or rewriting the query to avoid the skewed operation may help."; + } node.Warnings.Add(new PlanWarning { @@ -778,18 +761,37 @@ private static void AnalyzeNode(PlanNode node, PlanStatement stmt, AnalyzerConfi Severity = PlanWarningSeverity.Warning }); } - else if (!cfg.IsRuleDisabled(10) && node.Lookup && !string.IsNullOrEmpty(node.Predicate)) + else if (!cfg.IsRuleDisabled(10) && node.Lookup) { + var lookupMsg = "Key Lookup — SQL Server found rows via a nonclustered index but had to go back to the clustered index for additional columns."; + + // Show what columns the lookup is fetching + if (!string.IsNullOrEmpty(node.OutputColumns)) + lookupMsg += $"\nColumns fetched: {Truncate(node.OutputColumns, 200)}"; + + // Only call out the predicate if it actually filters rows + if (!string.IsNullOrEmpty(node.Predicate)) + { + var predicateFilters = node.HasActualStats && node.ActualExecutions > 0 + && node.ActualRows < node.ActualExecutions; + if (predicateFilters) + lookupMsg += $"\nResidual predicate (filtered {node.ActualExecutions - node.ActualRows:N0} rows): {Truncate(node.Predicate, 200)}"; + } + + lookupMsg += "\nTo eliminate the lookup, consider adding the needed columns as INCLUDE columns on the nonclustered index. This widens the index, so weigh the read benefit against write and storage overhead."; + node.Warnings.Add(new PlanWarning { WarningType = "Key Lookup", - Message = $"Key Lookup — SQL Server found rows via a nonclustered index but had to go back to the clustered index for additional columns. Alter the nonclustered index to add the predicate column as a key column or as an INCLUDE column.\nPredicate: {Truncate(node.Predicate, 200)}", + Message = lookupMsg, Severity = PlanWarningSeverity.Critical }); } // Rule 12: Non-SARGable predicate on scan - var nonSargableReason = cfg.IsRuleDisabled(12) ? null : DetectNonSargablePredicate(node); + // Skip for 0-execution nodes — the operator never ran, so the warning is academic + var nonSargableReason = cfg.IsRuleDisabled(12) || (node.HasActualStats && node.ActualExecutions == 0) + ? null : DetectNonSargablePredicate(node); if (nonSargableReason != null) { var nonSargableAdvice = nonSargableReason switch @@ -818,8 +820,9 @@ _ when nonSargableReason.StartsWith("Function call") => // Rule 11: Scan with residual predicate (skip if non-SARGable already flagged) // A PROBE() alone is just a bitmap filter — not a real residual predicate. + // Skip for 0-execution nodes — the operator never ran if (!cfg.IsRuleDisabled(11) && nonSargableReason == null && IsRowstoreScan(node) && !string.IsNullOrEmpty(node.Predicate) && - !IsProbeOnly(node.Predicate)) + !IsProbeOnly(node.Predicate) && !(node.HasActualStats && node.ActualExecutions == 0)) { var displayPredicate = StripProbeExpressions(node.Predicate); var details = BuildScanImpactDetails(node, stmt); @@ -902,6 +905,40 @@ _ when nonSargableReason.StartsWith("Function call") => } } + // Rule 34: Bare scan with narrow output — NC index or columnstore candidate. + // When a Clustered Index Scan or heap Table Scan reads the full table with no + // predicate but only outputs a few columns, a narrower nonclustered index could + // cover the query with far less I/O. For analytical workloads, columnstore may + // be a better fit. + var isBareScanCandidate = (node.PhysicalOp == "Clustered Index Scan" || node.PhysicalOp == "Table Scan") + && !node.Lookup + && string.IsNullOrEmpty(node.Predicate) + && !string.IsNullOrEmpty(node.OutputColumns); + if (!cfg.IsRuleDisabled(34) && isBareScanCandidate) + { + var colCount = node.OutputColumns!.Split(',').Length; + var isSignificant = node.HasActualStats + ? GetOperatorOwnElapsedMs(node) > 0 + : node.CostPercent >= 20; + + if (colCount <= 3 && isSignificant) + { + var scanKind = node.PhysicalOp == "Clustered Index Scan" + ? "Clustered index scan" + : "Heap table scan"; + var indexAdvice = node.PhysicalOp == "Clustered Index Scan" + ? "Consider a nonclustered index on the output columns (as key or INCLUDE) so SQL Server can read a narrower structure." + : "Consider a clustered or nonclustered index on the output columns so SQL Server can read a narrower structure."; + + node.Warnings.Add(new PlanWarning + { + WarningType = "Bare Scan", + Message = $"{scanKind} reads the full table with no predicate, outputting {colCount} column(s): {Truncate(node.OutputColumns, 200)}. {indexAdvice} For analytical workloads, a columnstore index may be a better fit.", + Severity = PlanWarningSeverity.Warning + }); + } + } + // Rule 13: Mismatched data types (GetRangeWithMismatchedTypes / GetRangeThroughConvert) if (!cfg.IsRuleDisabled(13) && node.PhysicalOp == "Compute Scalar" && !string.IsNullOrEmpty(node.DefinedValues)) { @@ -1119,7 +1156,7 @@ _ when nonSargableReason.StartsWith("Function call") => node.Warnings.Add(new PlanWarning { WarningType = "Top Above Scan", - Message = $"{topLabel} reads from {scanCandidate.PhysicalOp} (Node {scanCandidate.NodeId}).{innerNote}{predInfo} An index on the ORDER BY columns could eliminate the scan and sort entirely.", + Message = $"{topLabel} reads from {FormatNodeRef(scanCandidate)}.{innerNote}{predInfo} An index on the ORDER BY columns could eliminate the scan and sort entirely.", Severity = onInner ? PlanWarningSeverity.Critical : PlanWarningSeverity.Warning }); } @@ -1139,12 +1176,28 @@ _ when nonSargableReason.StartsWith("Function call") => // tiny floating-point differences that display identically are noise if (reduction >= 2.0) { - node.Warnings.Add(new PlanWarning + // If we have actual stats, check whether the row goal prediction was correct. + // When actual rows ≤ the row goal estimate, the optimizer stopped early as planned — benign. + var rowGoalWorked = false; + if (node.HasActualStats) { - WarningType = "Row Goal", - Message = $"Row goal active: estimate reduced from {node.EstimateRowsWithoutRowGoal:N0} to {node.EstimateRows:N0} ({reduction:N0}x reduction) due to TOP, EXISTS, IN, or FAST hint. The optimizer chose this plan shape expecting to stop reading early. If the query reads all rows anyway, the plan choice may be suboptimal.", - Severity = PlanWarningSeverity.Info - }); + var executions = node.ActualExecutions > 0 ? node.ActualExecutions : 1; + var actualPerExec = (double)node.ActualRows / executions; + rowGoalWorked = actualPerExec <= node.EstimateRows; + } + + if (!rowGoalWorked) + { + // Try to identify the specific row goal cause from the statement text + var cause = IdentifyRowGoalCause(stmt.StatementText); + + node.Warnings.Add(new PlanWarning + { + WarningType = "Row Goal", + Message = $"Row goal active: estimate reduced from {node.EstimateRowsWithoutRowGoal:N0} to {node.EstimateRows:N0} ({reduction:N0}x reduction) due to {cause}. The optimizer chose this plan shape expecting to stop reading early. If the query reads all rows anyway, the plan choice may be suboptimal.", + Severity = PlanWarningSeverity.Info + }); + } } } @@ -1166,7 +1219,8 @@ _ when nonSargableReason.StartsWith("Function call") => } // Rule 29: Enhance implicit conversion warnings — Seek Plan is more severe - if (!cfg.IsRuleDisabled(29)) + // Skip for 0-execution nodes — the operator never ran + if (!cfg.IsRuleDisabled(29) && !(node.HasActualStats && node.ActualExecutions == 0)) foreach (var w in node.Warnings.ToList()) { if (w.WarningType == "Implicit Conversion" && w.Message.StartsWith("Seek Plan")) @@ -1424,26 +1478,55 @@ private static bool IsOrExpansionChain(PlanNode concatenationNode) /// /// Finds Sort and Hash Match operators in the tree that consume memory. /// + /// + /// Returns true if the plan contains an adaptive join that executed as a Nested Loop. + /// Indicates a memory grant was sized for the hash alternative but never needed. + /// + private static bool HasAdaptiveJoinChoseNestedLoop(PlanNode node) + { + if (node.IsAdaptive && node.ActualJoinType != null + && node.ActualJoinType.Contains("Nested", StringComparison.OrdinalIgnoreCase)) + return true; + + foreach (var child in node.Children) + if (HasAdaptiveJoinChoseNestedLoop(child)) + return true; + + return false; + } + private static void FindMemoryConsumers(PlanNode node, List consumers) + { + // Collect all consumers first, then sort by row count descending + var raw = new List<(string Label, double Rows)>(); + FindMemoryConsumersRecursive(node, raw); + + foreach (var (label, _) in raw.OrderByDescending(c => c.Rows)) + consumers.Add(label); + } + + private static void FindMemoryConsumersRecursive(PlanNode node, List<(string Label, double Rows)> consumers) { if (node.PhysicalOp.Contains("Sort", StringComparison.OrdinalIgnoreCase) && !node.PhysicalOp.Contains("Spool", StringComparison.OrdinalIgnoreCase)) { + var rowCount = node.HasActualStats ? node.ActualRows : node.EstimateRows; var rows = node.HasActualStats ? $"{node.ActualRows:N0} actual rows" : $"{node.EstimateRows:N0} estimated rows"; - consumers.Add($"Sort (Node {node.NodeId}, {rows})"); + consumers.Add(($"Sort (Node {node.NodeId}, {rows})", rowCount)); } else if (node.PhysicalOp.Contains("Hash", StringComparison.OrdinalIgnoreCase)) { + var rowCount = node.HasActualStats ? node.ActualRows : node.EstimateRows; var rows = node.HasActualStats ? $"{node.ActualRows:N0} actual rows" : $"{node.EstimateRows:N0} estimated rows"; - consumers.Add($"Hash Match (Node {node.NodeId}, {rows})"); + consumers.Add(($"Hash Match (Node {node.NodeId}, {rows})", rowCount)); } foreach (var child in node.Children) - FindMemoryConsumers(child, consumers); + FindMemoryConsumersRecursive(child, consumers); } /// @@ -1707,90 +1790,6 @@ _ when wt.StartsWith("LCK_") => "lock contention", }; } - private static string? GetWaitStatsAdvice(List waits) - { - if (waits.Count == 0) - return null; - - var totalMs = waits.Sum(w => w.WaitTimeMs); - if (totalMs == 0) - return null; - - var top = waits.OrderByDescending(w => w.WaitTimeMs).First(); - var topPct = (double)top.WaitTimeMs / totalMs * 100; - - // Only give targeted advice if the dominant wait is >= 80% of total wait time - if (topPct < 80) - return null; - - return DescribeWaitType(top.WaitType, topPct); - } - - /// - /// Maps a wait type to a human-readable description with percentage context. - /// Covers all wait types observed in real execution plan files. - /// - private static string DescribeWaitType(string rawWaitType, double topPct) - { - var waitType = rawWaitType.ToUpperInvariant(); - return waitType switch - { - // I/O: reading/writing data pages from disk - _ when waitType.StartsWith("PAGEIOLATCH") => - $"I/O bound — {topPct:N0}% of wait time is {rawWaitType}. Data is being read from disk rather than memory. Consider adding indexes to reduce I/O, or investigate memory pressure.", - _ when waitType.Contains("IO_COMPLETION") => - $"I/O bound — {topPct:N0}% of wait time is {rawWaitType}. Non-buffer I/O such as sort/hash spills to TempDB or eager writes.", - - // CPU: thread yielding its scheduler quantum - _ when waitType == "SOS_SCHEDULER_YIELD" => - $"CPU bound — {topPct:N0}% of wait time is {rawWaitType}. The query is consuming significant CPU. Look for expensive operators (scans, sorts, hash builds) that could be eliminated or reduced.", - - // Parallelism: exchange and synchronization waits - _ when waitType.StartsWith("CXPACKET") || waitType.StartsWith("CXCONSUMER") => - $"Parallel thread skew — {topPct:N0}% of wait time is {rawWaitType}. Work is unevenly distributed across parallel threads.", - _ when waitType.StartsWith("CXSYNC") => - $"Parallel synchronization — {topPct:N0}% of wait time is {rawWaitType}. Threads are waiting at exchange operators to synchronize parallel execution.", - - // Hash operations - _ when waitType.StartsWith("HT") => - $"Hash operation — {topPct:N0}% of wait time is {rawWaitType}. Time spent building, repartitioning, or cleaning up hash tables. Large hash builds may indicate missing indexes or bad row estimates.", - - // Sort/bitmap batch operations - _ when waitType == "BPSORT" => - $"Batch sort — {topPct:N0}% of wait time is {rawWaitType}. Time spent in batch-mode sort operations.", - _ when waitType == "BMPBUILD" => - $"Bitmap build — {topPct:N0}% of wait time is {rawWaitType}. Time spent building bitmap filters for hash joins.", - - // Memory allocation - _ when waitType.Contains("MEMORY_ALLOCATION_EXT") => - $"Memory allocation — {topPct:N0}% of wait time is {rawWaitType}. Frequent memory allocations during query execution.", - - // Latch contention (non-I/O) - _ when waitType.StartsWith("PAGELATCH") => - $"Page latch contention — {topPct:N0}% of wait time is {rawWaitType}. In-memory page contention, often on TempDB or hot pages.", - _ when waitType.StartsWith("LATCH_") => - $"Latch contention — {topPct:N0}% of wait time is {rawWaitType}.", - - // Lock contention - _ when waitType.StartsWith("LCK_") => - $"Lock contention — {topPct:N0}% of wait time is {rawWaitType}. Other sessions are holding locks that this query needs.", - - // Log writes - _ when waitType == "LOGBUFFER" => - $"Log write — {topPct:N0}% of wait time is {rawWaitType}. Waiting for transaction log buffer flushes, typically from data modifications.", - - // Network - _ when waitType == "ASYNC_NETWORK_IO" => - $"Network bound — {topPct:N0}% of wait time is {rawWaitType}. The client application is not consuming results fast enough.", - - // Physical page cache - _ when waitType == "SOS_PHYS_PAGE_CACHE" => - $"Physical page cache — {topPct:N0}% of wait time is {rawWaitType}. Contention on the physical memory page allocator.", - - _ => $"Dominant wait is {rawWaitType} ({topPct:N0}% of wait time)." - }; - } - /// /// Returns true if the statement has significant I/O waits (PAGEIOLATCH_*, IO_COMPLETION). /// Used for severity elevation decisions where I/O specifically indicates disk access. @@ -1918,6 +1917,50 @@ private static bool HasSpillWarning(PlanNode node) return node.Warnings.Any(w => w.SpillDetails != null); } + /// + /// Formats a node reference for use in warning messages. Includes object name + /// for data access operators where it helps identify which table is involved. + /// + private static string FormatNodeRef(PlanNode node) + { + if (!string.IsNullOrEmpty(node.ObjectName)) + { + var objRef = !string.IsNullOrEmpty(node.DatabaseName) + ? $"{node.DatabaseName}.{node.ObjectName}" + : node.ObjectName; + return $"{node.PhysicalOp} on {objRef} (Node {node.NodeId})"; + } + + return $"{node.PhysicalOp} (Node {node.NodeId})"; + } + + /// + /// Identifies the specific cause of a row goal from the statement text. + /// Returns a specific cause when detectable, or a generic list as fallback. + /// + private static string IdentifyRowGoalCause(string stmtText) + { + if (string.IsNullOrEmpty(stmtText)) + return "TOP, EXISTS, IN, or FAST hint"; + + var text = stmtText.ToUpperInvariant(); + var causes = new List(4); + + if (Regex.IsMatch(text, @"\bTOP\b")) + causes.Add("TOP"); + if (Regex.IsMatch(text, @"\bEXISTS\b")) + causes.Add("EXISTS"); + // IN with subquery — bare "IN (" followed by SELECT, not just "IN (1,2,3)" + if (Regex.IsMatch(text, @"\bIN\s*\(\s*SELECT\b")) + causes.Add("IN (subquery)"); + if (Regex.IsMatch(text, @"\bFAST\b")) + causes.Add("FAST hint"); + + return causes.Count > 0 + ? string.Join(", ", causes) + : "TOP, EXISTS, IN, or FAST hint"; + } + private static string Truncate(string value, int maxLength) { return value.Length <= maxLength ? value : value[..maxLength] + "..."; diff --git a/src/PlanViewer.Web/Pages/Index.razor b/src/PlanViewer.Web/Pages/Index.razor index 669efb5..3a54c88 100644 --- a/src/PlanViewer.Web/Pages/Index.razor +++ b/src/PlanViewer.Web/Pages/Index.razor @@ -160,6 +160,14 @@ else CPU @ActiveStmt!.QueryTime.CpuTimeMs.ToString("N0") ms + @if (ActiveStmt!.QueryTime.ElapsedTimeMs > 0) + { + var ratio = (double)ActiveStmt!.QueryTime.CpuTimeMs / ActiveStmt!.QueryTime.ElapsedTimeMs; +
+ CPU:Elapsed + @ratio.ToString("N2") +
+ } } @if (ActiveStmt!.DegreeOfParallelism > 0) { @@ -280,15 +288,23 @@ else @if (ActiveStmt!.WaitStats.Count > 0) { var maxWait = ActiveStmt!.WaitStats.Max(w => w.WaitTimeMs); + var benefitLookup = ActiveStmt!.WaitBenefits.ToDictionary(wb => wb.WaitType, wb => wb.MaxBenefitPercent, StringComparer.OrdinalIgnoreCase); @foreach (var w in ActiveStmt!.WaitStats.OrderByDescending(w => w.WaitTimeMs)) { var barPct = maxWait > 0 ? (double)w.WaitTimeMs / maxWait * 100 : 0; + benefitLookup.TryGetValue(w.WaitType, out var benefitPct);
@w.WaitType
- @w.WaitTimeMs.ToString("N0") ms + + @w.WaitTimeMs.ToString("N0") ms + @if (benefitPct > 0) + { + up to @benefitPct.ToString("N0")% + } +
} } diff --git a/src/PlanViewer.Web/wwwroot/css/app.css b/src/PlanViewer.Web/wwwroot/css/app.css index b352858..f512855 100644 --- a/src/PlanViewer.Web/wwwroot/css/app.css +++ b/src/PlanViewer.Web/wwwroot/css/app.css @@ -717,6 +717,16 @@ textarea::placeholder { font-size: 0.7rem; } +.wait-benefit { + font-size: 0.65rem; + font-weight: 600; + color: var(--text-secondary); + padding: 0.05rem 0.3rem; + border-radius: 3px; + background: rgba(0, 0, 0, 0.04); + margin-left: 0.25rem; +} + /* === Warnings Strip === */ .warnings-strip { margin-bottom: 0.75rem; diff --git a/tests/PlanViewer.Core.Tests/PlanAnalyzerTests.cs b/tests/PlanViewer.Core.Tests/PlanAnalyzerTests.cs index 5ca3373..8944a39 100644 --- a/tests/PlanViewer.Core.Tests/PlanAnalyzerTests.cs +++ b/tests/PlanViewer.Core.Tests/PlanAnalyzerTests.cs @@ -94,13 +94,18 @@ public void Rule05_RowEstimateMismatch_FalsePositivesSuppressed() // --------------------------------------------------------------- [Fact] - public void Rule06_ScalarUdfReference_DetectsUdfInPlan() + public void Rule06_ScalarUdfReference_SuppressedWhenSerialPlanCoversIt() { + // The udf_plan has NonParallelPlanReason = TSQLUserDefinedFunctionsNotParallelizable, + // so the Serial Plan warning already explains why the plan is serial and Rule 6 + // would be redundant (per Joe's b6 feedback on #215). var plan = PlanTestHelper.LoadAndAnalyze("udf_plan.sqlplan"); - var warnings = PlanTestHelper.WarningsOfType(plan, "Scalar UDF"); + var udfWarnings = PlanTestHelper.WarningsOfType(plan, "Scalar UDF"); + var serialWarnings = PlanTestHelper.WarningsOfType(plan, "Serial Plan"); - Assert.NotEmpty(warnings); - Assert.Contains(warnings, w => w.Message.Contains("once per row")); + Assert.Empty(udfWarnings); + Assert.NotEmpty(serialWarnings); + Assert.Contains(serialWarnings, w => w.Message.Contains("UDF")); } // --------------------------------------------------------------- @@ -573,32 +578,8 @@ public void Rule29_ImplicitConvertSeekPlan_UpgradedToCritical() Assert.Contains(warnings, w => w.Message.Contains("prevented an index seek")); } - // --------------------------------------------------------------- - // Rule 25: Ineffective Parallelism - // --------------------------------------------------------------- - - [Fact] - public void Rule25_IneffectiveParallelism_DetectedWhenCpuEqualsElapsed() - { - // serially-parallel: DOP 8 but CPU 17,110ms ≈ elapsed 17,112ms (efficiency ~0%) - var plan = PlanTestHelper.LoadAndAnalyze("serially-parallel.sqlplan"); - var warnings = PlanTestHelper.WarningsOfType(plan, "Ineffective Parallelism"); - - Assert.Single(warnings); - Assert.Contains("DOP 8", warnings[0].Message); - Assert.Contains("% efficient", warnings[0].Message); - } - - [Fact] - public void Rule25_IneffectiveParallelism_NotFiredOnEffectiveParallelPlan() - { - // parallel-skew: DOP 4, CPU 28,634ms vs elapsed 9,417ms (ratio ~3.0) - // This is effective parallelism — Rule 25 should NOT fire - var plan = PlanTestHelper.LoadAndAnalyze("parallel-skew.sqlplan"); - var warnings = PlanTestHelper.WarningsOfType(plan, "Ineffective Parallelism"); - - Assert.Empty(warnings); - } + // Rules 25 and 31 were removed — CPU:Elapsed ratio is shown in the runtime + // summary instead, and wait stats speak for themselves. // --------------------------------------------------------------- // Rule 28: NOT IN with Nullable Column (Row Count Spool) @@ -642,25 +623,6 @@ public void Rule30_MissingIndexQuality_DetectsWideOrLowImpact() } } - // --------------------------------------------------------------- - // Rule 31: Parallel Wait Bottleneck - // --------------------------------------------------------------- - - [Fact] - public void Rule31_ParallelWaitBottleneck_DetectedWhenElapsedExceedsCpu() - { - // excellent-parallel-spill: DOP 4, CPU 172,222ms vs elapsed 225,870ms - // speedup ~0.76 — CPU < Elapsed but >= 0.5, so fires as Ineffective Parallelism - // (wait bottleneck only fires when speedup < 0.5 — extreme waiting) - var plan = PlanTestHelper.LoadAndAnalyze("excellent-parallel-spill.sqlplan"); - - // At DOP 4 with speedup 0.76, efficiency ≈ 0% — fires Ineffective Parallelism - var warnings = PlanTestHelper.WarningsOfType(plan, "Ineffective Parallelism"); - Assert.NotEmpty(warnings); - Assert.Contains("DOP 4", warnings[0].Message); - Assert.Contains("% efficient", warnings[0].Message); - } - // --------------------------------------------------------------- // Seek Predicate Parsing // ---------------------------------------------------------------