perf: skip metric name normalization for symbol inputs - #1
Open
artl93 wants to merge 1 commit into
Open
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2e2e074c-44dc-42cd-978f-31553d7876dd
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
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.
Purpose
FromMetriccurrently scans and replaces every supported metric-prefix name on every call—even when the input is already a plain number or uses a metric symbol such asM. That unnecessary normalization creates several intermediate strings and dominates these common parsing paths.This focused change avoids name normalization when the trimmed input already ends in a numeric value or a recognized standalone symbol. Name-based inputs still use the existing replacement path, including ambiguous cases such as
megaversus theasymbol.Changes
mega, attacheda, and spacedainputs.The earlier 158-file sealing, LINQ, title-casing, and devirtualization proposal was removed. The PR now changes only
MetricNumeralExtensionsand its tests.Benchmark methodology
Clean
main(f9292aa90948de0aea2d4fa7d6549b1b2432c0fb) was compared with this commit (fdf80d51ee78d32f0e1a0430c72f687076c94537) using a temporary cross-build BenchmarkDotNet harness. It deliberately does not use the repository benchmark project's globalInProcessEmitToolchainconfiguration.MemoryDiagnoserThe relevant job configuration was:
Command:
Results
"123.45M".FromMetric()"123.45 M".FromMetric()"123.45 mega".FromMetric()"123.45".FromMetric()ToMetricwith symbolToMetricwith symbolToMetricwith nameThe common symbol and plain-number
FromMetricpaths improve by 58–69% while eliminating 384 B per call. Name parsing improves by 11% and also removes 384 B.ToMetriccases remain neutral to slightly faster. No unrelated ratios are averaged, and no measured case regressed.Disassembly is not relevant to the retained mechanism: the improvement comes from skipping normalization work and allocations, not from devirtualization or inlining.
Validation
dotnet format --verify-no-changesThe exact documented pack command without
RunAnalyzers=falseis currently blocked by pre-existingIDE0055errors in unchanged clean-main filesEnumDehumanizeExtensions.csandPolyfillShims.cs.