Repository navigation
Plan viewer: a shared MetricFormatter for costs and durations - #4592
Merged
Merged
Conversation
Ports erikdarlingdata/PerformanceStudio@caa57ce's MetricFormatter.cs and its tests to PerformanceMonitor.PlanAnalysis, and routes the properties panel's Estimated Costs and the node tooltips' cost rows through it, replacing raw ":F6" formatting that showed costs like "16.765900" or "0.000000". FormatCost caps a cost at four decimals, strips trailing zeros, adds thousands separators, shows an exact zero as "0", and shows a non-zero cost too small for four decimals as "<0.0001" rather than rounding it away to "0". It never falls back to scientific notation. FormatDuration keeps the existing three-rung ladder (ms under a second, seconds with one decimal under a minute, minutes+seconds beyond that) and adds a double overload with two rungs below a millisecond, so a sub-millisecond duration reads as e.g. "0.4ms" instead of rounding to "0ms". The statements grid's private FormatDuration(long) now delegates to the shared helper instead of keeping its own copy of the same ladder. MemoryFractionInput/Output keep their own ":F4" formatting; that's a different metric family this change leaves alone. A census test asserts no ":F6" cost formatting is left anywhere in PerformanceMonitor.Ui.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #4577, Part of #4511
Why
erikdarlingdata/PerformanceStudio@caa57ce (dev at 85492a1) added
MetricFormatterand routed every raw numeric cost/duration display through it. The plan viewer is shared between the Darling Viewer and Lite, and until this PR it still formatted optimizer costs with a raw:F6, so the properties panel and node tooltips showed strings like16.765900or0.000000instead of a trimmed, thousands-separated number.What changes
PerformanceMonitor.PlanAnalysis/MetricFormatter.cs, ported near-verbatim from PerformanceStudio'sPlanViewer.Core/Output/MetricFormatter.cs:FormatCost: at most four decimal places, trailing zeros stripped, thousands separators, exact zero shown as"0", a non-zero cost too small for four decimals shown as"<0.0001"rather than rounding away to"0", never scientific notation.FormatDuration(long): the existing three-rung ladder (ms under a second, seconds with one decimal under a minute, minutes+seconds beyond that).FormatDuration(double): two more rungs below one millisecond (e.g."0.4ms","<0.01ms"), delegating to the long overload at and above 1ms.PlanViewerControl.Properties.cs(Operator/Subtree/I/O/CPU Cost rows) andPlanViewerControl.Tooltips.cs(the same four cost rows) now callMetricFormatter.FormatCostinstead of:F6.PlanViewerControl.xaml.cs'sStatementRow.FormatDuration(long)private helper is removed;CpuDisplay/ElapsedDisplay/UdfDisplaynow callMetricFormatter.FormatDurationdirectly.CpuMs/ElapsedMs/UdfMsarelong(showplan'sCpuTime/ElapsedTime/UdfElapsedTimeXML attributes parse tolonginShowPlanParser), so the sub-milliseconddoubleoverload has no call site in the statements grid; it's ported for the Query Store path PerformanceStudio's issue names, which PM doesn't yet have wired to a display.CostDisplay(EstCost:F2, the grid's estimated-subtree-cost column) is unchanged: PerformanceStudio's routing didn't touch its own equivalent statements-grid cost column shape (F2, notF6), and it isn't one of the four Operator/Subtree/I/O/CPU cost rows this issue calls out.MemoryFractionInput/MemoryFractionOutput(:F4) are a different metric family PerformanceStudio leaves alone; unchanged.Lite: gets this too (shared control) —
PlanViewerControlis hosted by both the Darling Viewer and Lite.Not ported: PerformanceStudio's comparison-report and web-viewer routing sites have no PM equivalent (no comparison report, no Blazor/web viewer).
Test plan
New
Darling/Darling.Tests/Viewer4577Tests.cs, ported from PerformanceStudio'sMetricFormatterTests.cs(all cases, all InvariantCulture-pinned), plus a census test (NoRawCostFormatStringSurvivesInTheUi) that walksPerformanceMonitor.Ui/*.csand fails if any file still contains:F6.EnableWindowsTargeting=true, Release):Darling.Tests,PerformanceMonitor.Ui,Lite/PerformanceMonitorLite.csproj,Lite.Tests,Darling/PerformanceMonitor.Darling.Viewer— all 0 warnings / 0 errors.git worktree add --detach <pre-fix sha>): copyingViewer4577Tests.csin withoutMetricFormatter.csis a compile failure (CS0103: The name 'MetricFormatter' does not exist) — expected, since the class is new. A second, runtime-only copy of just the census assertion (noMetricFormatterreference) run against the pre-fixPlanViewerControl.Tooltips.cs/Properties.csfails at runtime:Raw ":F6" cost formatting left in: .../PlanViewerControl.Tooltips.cs, .../PlanViewerControl.Properties.cs.FormatCost'scost == 0branch to return"MUTATED";FormatCost_ShapesTheNumber(cost: 0, expected: "0")failed as expected; reverted; rebuilt clean.Microsoft.WindowsDesktop.Appstripped from runtimeconfig):Viewer4577Tests(46 cases) plusDocCommentHygieneTests,ShowPlanParserCondAndMultiplePlanTests,ActualPlanRequestTests,ActualPlanDispatchTests,ActualPlanResultParseTests,QueryModificationDetectorTests,ActualPlanCaptureLoopTests,ActualPlanGatingTests,ReproScriptBuilderHardeningTests,DarlingAnalysisPipelineTests,DarlingMcpPlanToolsSurfaceAndSqlTests,McpPlanAnalysisEnvelopeTests,SerialLoopStoreSizeSourceTests,TsqlConventionGuardTests:Total: 276, Errors: 0, Failed: 0, Skipped: 1, Not Run: 0(DarlingMcpPlanToolsLivePostgresTestsskips — no local Postgres rig).Darling.Tests/Lite.Teststargetnet10.0-windows: they build on macOS but Lite.Tests cannot run here; CI decides Lite.Tests. No Lite.Tests file was touched by this change, so nothing new needs a Windows-only read.16.7659or a tiny cost like0.00002), check the Properties panel's Operator/Subtree/I/O/CPU Cost rows read as trimmed numbers with thousands separators (not16.765900), and hover a node to confirm the tooltip's Cost/Subtree Cost/I/O Cost/CPU Cost rows match the same formatting.CHANGELOG
SECTION: Changed
ENTRY: - Plan viewer costs are formatted as in PerformanceStudio ([#4592]) - Costs in the properties panel and node tooltips show a trimmed, thousands-separated number instead of six raw decimal places. The sub-millisecond duration format is ported too, but the viewer's statement times are whole milliseconds today, so nothing shows it yet.
REF: [#4592]: #4592