Modernization: stack-overflow fix, frozen collections, drop net9.0, centralized build config by twcclegg · Pull Request #370 · twcclegg/libphonenumber-csharp

twcclegg · 2026-08-03T22:51:38Z

Thirteen commits, worst-first. Measured on the parse path: ~2% faster
(ParseValidateAndFormat −1.6% at 1000 numbers, −2.6% at 10000, both sides measured on the same
runner), costing +18.25 KB one-time allocation per PhoneNumberUtil with per-parse allocation
unchanged.

Correctness

fix: stop oversized input overflowing the stack in Normalize — Normalize,
NormalizeDigitsOnly, NormalizeDiallableCharsOnly and ConvertAlphaCharactersInNumber did
stackalloc char[number.Length] on caller-supplied strings with no length guard, so a large input
killed the process with an uncatchable StackOverflowException. Parse has
MAX_INPUT_STRING_LENGTH = 250; these four public statics had nothing. Buffers over 256 chars now
come from ArrayPool. The buffer stays sliced to exactly number.Length — the normalizers rely on
running out of room to drop characters whose numeric value needs more digits than the character it
came from. netstandard2.0 uses StringBuilder and was never affected.

fix: return a copy from GetSupportedRegions and freeze the region sets — the getter returned
the instance's own HashSet<string>, and PhoneNumberUtil is a process-wide singleton, so any
caller could Remove() a region and break validation for the rest of the process. Java returns
Collections.unmodifiableSet here. The return type can't change without a binary break (and
IReadOnlySet doesn't exist on netstandard2.0), so it returns a copy; the method has no internal
callers. That also unblocked freezing supportedRegions and nanpaRegions.

fix: stop parallel inner builds racing on clean of the generated bins — dotnet clean on a
cross-targeting project dispatches Clean to each inner TFM build, so three RemoveDir calls raced
on one shared obj/metadata and failed with MSB3231. Pre-existing; nothing in CI ran
dotnet clean until the determinism step below. Both clean targets now run on the outer build only.

fix: copy the region code map on all targets and reject unknown normalize modes — self-review
fixes: the frozen-map change had made net8+ copy the constructor's dictionary while netstandard2.0
kept aliasing it, and the mode switch would have silently returned "" for an unhandled case.

Performance

perf: use frozen collections for the short number and timezone lookups — FrozenSet for
ShortNumbersRegionCodeSet and FrozenDictionary for the timezone map on net8+, plus ContainsKey

indexer collapsed to one TryGetValue in LookUpPrefix. That loop runs once per digit, so it was
up to ~24 probes of an ImmutableDictionary per query; the TryGetValue half helps every TFM.

perf: freeze the country calling code to region code map — int keys, ~215 entries, read on
every parse (ContainsKey up to 3× in MaybeExtractCountryCode, plus TryGetValue in
IsValidNumber, GetRegionCodeForCountryCode, HasValidCountryCallingCode). Behind a file-scoped
type alias so the per-target difference sits in one place.

perf: size the normalize stack buffer to the input — stackalloc zero-initializes, so the
fixed 256-char buffer from the first commit cleared 512 bytes on every call where the original
cleared only number.Length * 2.

Build and CI

chore: drop the out-of-support net9.0 target — there are zero NET8_0/NET9_0/NET10_0
conditionals, so those three assets compiled from identical source. .NET 9 left support 2026-05-12;
net9 consumers resolve the net8.0 asset. Cuts ~25% off build and the test matrix.

chore: centralize build settings and package versions — Directory.Build.props plus Central
Package Management, collapsing dependabot's seven nuget entries to one. LangVersion goes
preview → latest so an SDK bump can't change the semantics of already-shipped code. Both demo
workflows' paths: filters gained the props files, or they'd inherit settings no PR job builds.

chore: keep nuget audit findings from failing the build — NU1901–NU1904 become warnings
and NuGetAuditMode=all covers transitives. Otherwise a newly published advisory against the
netstandard2.0-only deps fails the release build days after a commit that touched nothing.

build: pin the sdk major and verify builds are reproducible — global.json pins .NET 10 with
allowPrerelease: false. New CI step packs, cleans, packs again and compares assembly hashes;
cleaning also drops the generated metadata bins, so it covers MetadataBuilder's output too.

ci: benchmark the base commit on the same runner as the branch — the PR base is checked into a
worktree and measured in the same job. The cached baseline came from a different machine, and
cross-machine variance is a few percent — the same size as the effects being measured, which is why
the previous run reported the two ParseValidateAndFormat sizes moving in opposite directions.
Removing the cache also removes the cron that existed only to keep it inside GitHub's 7-day eviction
window. ~10 min per PR instead of ~5. Missing base results now fail the job rather than posting a
one-sided comment.

refactor: guard nullable annotations on netstandard2.0 rather than a net version — six
#if NET6_0_OR_GREATER blocks only selected between T? and T signatures, tracking the csproj's
<Nullable> condition rather than any API floor. They now say !NETSTANDARD2_0, which is what the
csproj actually tests. The three NET6 guards that do gate real APIs (string.Concat(ReadOnlySpan),
StringBuilder.Append(StringBuilder, int, int), HashSet<T>(int)) are unchanged, as are the NET7
[GeneratedRegex] and NET8 Frozen guards.

Reviewer notes

Benchmarks can't resolve small deltas even on one runner. ExtractPossibleNumber_CleanInput
moved ±4% between runs despite this PR not touching it — code layout and JIT effects. Read the
parse result as credible because both input sizes agree in direction and magnitude, not because a
single delta cleared its error bars. The ColdStart* benchmarks carry ±35% error and can't
support any conclusion.
GetRegionCodesForCountryCode still returns the internal List<string> as
IReadOnlyList<string> — safe unless a caller casts back, where Java uses unmodifiableList.
Left alone because it is on an internal path (PhoneNumberOfflineGeocoder.cs:137), so guarding
it costs a wrapper allocation per geocode.
The determinism check proves same-machine reproducibility only. It won't catch path dependence
or filesystem enumeration order.

Deliberately not done

LocaleData.Data — nested ImmutableDictionary, two slow probes per geocode call, the largest
read win left. Needs the change in lib/DumpLocale.java rather than the generated file, and
freezing something that size is where construction cost bites. Separate PR so its numbers isolate.
AnalysisMode on the main library (weaker than the Extensions helper today) — will produce a
pile of CA warnings on the ported core, which TreatWarningsAsErrors turns into a build break.
Sorting the EmbeddedResource globs — MSBuild may already sort glob results; confirming needs
a cross-machine comparison.
Broad C# syntax modernization in the ported core — makes future upstream Java diffs harder to
read for cosmetic gain.

codecov · 2026-08-03T22:54:14Z

Codecov Report

❌ Patch coverage is 93.02326% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 98.00%. Comparing base (4a355f1) to head (b4e9a99).

Files with missing lines	Patch %	Lines
csharp/PhoneNumbers/PhoneNumberUtil.net.cs	91.30%	1 Missing and 1 partial ⚠️
csharp/PhoneNumbers/PhoneNumberUtil.cs	90.90%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #370      +/-   ##
==========================================
- Coverage   98.01%   98.00%   -0.01%     
==========================================
  Files          39       39              
  Lines       52792    52798       +6     
  Branches     1097     1101       +4     
==========================================
+ Hits        51743    51747       +4     
- Misses        796      797       +1     
- Partials      253      254       +1

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

github-actions · 2026-08-03T22:57:04Z

📊 Benchmark Results

Commit: b4e9a99 · Full run · Linux ubuntu-24.04-arm

PR branch


BenchmarkDotNet v0.15.8, Linux Ubuntu 24.04.4 LTS (Noble Numbat)
Neoverse-N2, 4 physical cores
.NET SDK 10.0.302
  [Host]    : .NET 10.0.10 (10.0.10, 10.0.1026.32716), Arm64 RyuJIT armv8.0-a
  .NET 10.0 : .NET 10.0.10 (10.0.10, 10.0.1026.32716), Arm64 RyuJIT armv8.0-a

Job=.NET 10.0  Runtime=.NET 10.0

Method	PhoneNumberCount	Mean	Error	StdDev	Gen0	Allocated
InputDigitPerKeystroke	1000	4.472 ms	0.0877 ms	0.1044 ms	54.6875	3.87 MB


BenchmarkDotNet v0.15.8, Linux Ubuntu 24.04.4 LTS (Noble Numbat)
Neoverse-N2, 4 physical cores
.NET SDK 10.0.302
  [Host]     : .NET 10.0.10 (10.0.10, 10.0.1026.32716), Arm64 RyuJIT armv8.0-a
  Job-AMQORM : .NET 10.0.10 (10.0.10, 10.0.1026.32716), Arm64 RyuJIT armv8.0-a

Runtime=.NET 10.0  InvocationCount=1  IterationCount=20  
LaunchCount=1  RunStrategy=ColdStart  UnrollFactor=1  
WarmupCount=1

Method	Mean	Error	StdDev	Allocated
CreateInstance	396.2 μs	115.9 μs	133.4 μs	119.48 KB
CreateInstanceAndLoadAllRegions	7,060.0 μs	387.2 μs	445.9 μs	1619.1 KB
FirstRegionLookup	470.0 μs	153.0 μs	176.2 μs	124.54 KB


BenchmarkDotNet v0.15.8, Linux Ubuntu 24.04.4 LTS (Noble Numbat)
Neoverse-N2, 4 physical cores
.NET SDK 10.0.302
  [Host]    : .NET 10.0.10 (10.0.10, 10.0.1026.32716), Arm64 RyuJIT armv8.0-a
  .NET 10.0 : .NET 10.0.10 (10.0.10, 10.0.1026.32716), Arm64 RyuJIT armv8.0-a

Job=.NET 10.0  Runtime=.NET 10.0

Method	PhoneNumberCount	Mean	Error	StdDev	Gen0	Allocated
ExtractPossibleNumber_CleanInput	1000	20.89 μs	0.018 μs	0.017 μs	-	-
ExtractPossibleNumber_WithLeadingJunk	1000	38.38 μs	0.079 μs	0.074 μs	0.6714	48360 B


BenchmarkDotNet v0.15.8, Linux Ubuntu 24.04.4 LTS (Noble Numbat)
Neoverse-N2, 4 physical cores
.NET SDK 10.0.302
  [Host]    : .NET 10.0.10 (10.0.10, 10.0.1026.32716), Arm64 RyuJIT armv8.0-a
  .NET 10.0 : .NET 10.0.10 (10.0.10, 10.0.1026.32716), Arm64 RyuJIT armv8.0-a

Job=.NET 10.0  Runtime=.NET 10.0

Method	PhoneNumberCount	Mean	Error	StdDev	Gen0	Allocated
FindNumbers_Valid	100	139.3 μs	0.33 μs	0.29 μs	0.9766	69.93 KB
FindNumbers_StrictGrouping	100	304.8 μs	0.63 μs	0.59 μs	1.4648	123.2 KB


BenchmarkDotNet v0.15.8, Linux Ubuntu 24.04.4 LTS (Noble Numbat)
Neoverse-N2, 4 physical cores
.NET SDK 10.0.302
  [Host]    : .NET 10.0.10 (10.0.10, 10.0.1026.32716), Arm64 RyuJIT armv8.0-a
  .NET 10.0 : .NET 10.0.10 (10.0.10, 10.0.1026.32716), Arm64 RyuJIT armv8.0-a

Job=.NET 10.0  Runtime=.NET 10.0

Method	PhoneNumberCount	Mean	Error	StdDev	Gen0	Allocated
ParseValidateAndFormatPhoneNumbers	1000	2.490 ms	0.0128 ms	0.0107 ms	7.8125	580.51 KB
ParseValidateAndFormatPhoneNumbers	10000	25.636 ms	0.1419 ms	0.1185 ms	62.5000	5798.53 KB

main branch


BenchmarkDotNet v0.15.8, Linux Ubuntu 24.04.4 LTS (Noble Numbat)
Neoverse-N2, 4 physical cores
.NET SDK 10.0.302
  [Host]    : .NET 10.0.10 (10.0.10, 10.0.1026.32716), Arm64 RyuJIT armv8.0-a
  .NET 10.0 : .NET 10.0.10 (10.0.10, 10.0.1026.32716), Arm64 RyuJIT armv8.0-a

Job=.NET 10.0  Runtime=.NET 10.0

Method	PhoneNumberCount	Mean	Error	StdDev	Gen0	Allocated
InputDigitPerKeystroke	1000	4.391 ms	0.0092 ms	0.0086 ms	54.6875	3.87 MB


BenchmarkDotNet v0.15.8, Linux Ubuntu 24.04.4 LTS (Noble Numbat)
Neoverse-N2, 4 physical cores
.NET SDK 10.0.302
  [Host]     : .NET 10.0.10 (10.0.10, 10.0.1026.32716), Arm64 RyuJIT armv8.0-a
  Job-AMQORM : .NET 10.0.10 (10.0.10, 10.0.1026.32716), Arm64 RyuJIT armv8.0-a

Runtime=.NET 10.0  InvocationCount=1  IterationCount=20  
LaunchCount=1  RunStrategy=ColdStart  UnrollFactor=1  
WarmupCount=1

Method	Mean	Error	StdDev	Allocated
CreateInstance	322.5 μs	98.04 μs	112.9 μs	75.05 KB
CreateInstanceAndLoadAllRegions	6,927.2 μs	338.03 μs	389.3 μs	1575.98 KB
FirstRegionLookup	368.7 μs	129.30 μs	148.9 μs	80.11 KB


BenchmarkDotNet v0.15.8, Linux Ubuntu 24.04.4 LTS (Noble Numbat)
Neoverse-N2, 4 physical cores
.NET SDK 10.0.302
  [Host]    : .NET 10.0.10 (10.0.10, 10.0.1026.32716), Arm64 RyuJIT armv8.0-a
  .NET 10.0 : .NET 10.0.10 (10.0.10, 10.0.1026.32716), Arm64 RyuJIT armv8.0-a

Job=.NET 10.0  Runtime=.NET 10.0

Method	PhoneNumberCount	Mean	Error	StdDev	Gen0	Allocated
ExtractPossibleNumber_CleanInput	1000	21.15 μs	0.015 μs	0.014 μs	-	-
ExtractPossibleNumber_WithLeadingJunk	1000	38.29 μs	0.032 μs	0.028 μs	0.6714	48360 B


BenchmarkDotNet v0.15.8, Linux Ubuntu 24.04.4 LTS (Noble Numbat)
Neoverse-N2, 4 physical cores
.NET SDK 10.0.302
  [Host]    : .NET 10.0.10 (10.0.10, 10.0.1026.32716), Arm64 RyuJIT armv8.0-a
  .NET 10.0 : .NET 10.0.10 (10.0.10, 10.0.1026.32716), Arm64 RyuJIT armv8.0-a

Job=.NET 10.0  Runtime=.NET 10.0

Method	PhoneNumberCount	Mean	Error	StdDev	Gen0	Allocated
FindNumbers_Valid	100	138.8 μs	0.22 μs	0.18 μs	0.9766	69.93 KB
FindNumbers_StrictGrouping	100	306.9 μs	0.69 μs	0.65 μs	1.4648	123.2 KB


BenchmarkDotNet v0.15.8, Linux Ubuntu 24.04.4 LTS (Noble Numbat)
Neoverse-N2, 4 physical cores
.NET SDK 10.0.302
  [Host]    : .NET 10.0.10 (10.0.10, 10.0.1026.32716), Arm64 RyuJIT armv8.0-a
  .NET 10.0 : .NET 10.0.10 (10.0.10, 10.0.1026.32716), Arm64 RyuJIT armv8.0-a

Job=.NET 10.0  Runtime=.NET 10.0

Method	PhoneNumberCount	Mean	Error	StdDev	Gen0	Allocated
ParseValidateAndFormatPhoneNumbers	1000	2.548 ms	0.0065 ms	0.0061 ms	7.8125	580.51 KB
ParseValidateAndFormatPhoneNumbers	10000	25.705 ms	0.2723 ms	0.2547 ms	62.5000	5798.53 KB

…net version

…lize modes

wmundev

nice changes, looks good!

…MapFromString 5 cases used a leftover try { ...; Assert.True(false); } catch (Exception) { } pattern where every other case in the same method already uses the cleaner Assert.Throws<Exception>(() => ...) idiom for the exact same kind of assertion. Made these 5 consistent with the rest (cs/catch-of-all-exceptions, alerts #279-#283). Left TestEquals_WhenNull_ReturnsFalse alone (cs/null-argument-to-equals, alert #370): MetadataFilter.Equals uses `obj is not MetadataFilter other` pattern matching, which handles a null argument safely (no NRE) - the test is correctly verifying that exact contract, not an accidental risky Equals(null) call. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

twcclegg added 6 commits August 3, 2026 13:23

fix: stop oversized input overflowing the stack in Normalize

7650324

perf: use frozen collections for the short number and timezone lookups

102da88

chore: drop the out-of-support net9.0 target

4b766ff

chore: centralize build settings and package versions

ce8ad17

chore: keep nuget audit findings from failing the build

801370b

build: pin the sdk major and verify builds are reproducible

124d08e

twcclegg added 7 commits August 3, 2026 19:32

perf: size the normalize stack buffer to the input

852a0fc

fix: stop parallel inner builds racing on clean of the generated bins

095ac82

perf: freeze the country calling code to region code map

8f45303

ci: benchmark the base commit on the same runner as the branch

354feaa

fix: return a copy from GetSupportedRegions and freeze the region sets

bd478fa

refactor: guard nullable annotations on netstandard2.0 rather than a …

e16c344

…net version

fix: copy the region code map on all targets and reject unknown norma…

b4e9a99

…lize modes

twcclegg requested a review from wmundev August 4, 2026 17:11

wmundev approved these changes Aug 5, 2026

View reviewed changes

twcclegg merged commit 7f536e9 into main Aug 5, 2026
8 checks passed

twcclegg mentioned this pull request Aug 5, 2026

Tracking: improvements blocked on a major version rev #375

Open

This was referenced Aug 23, 2026

Bump the nuget-minor-and-patch group with 27 updates linkuity/linkuity#91

Closed

Bump the production-dependencies group with 35 updates quannv108/clean-architecture-template#86

Closed

dependabot Bot mentioned this pull request Sep 4, 2026

Bump the dotnet-deps group with 16 updates vodongha/vodongha-personal#321

Open

Navigation Menu

Modernization: stack-overflow fix, frozen collections, drop net9.0, centralized build config - #370

Modernization: stack-overflow fix, frozen collections, drop net9.0, centralized build config#370
twcclegg merged 13 commits into
mainfrom
modernize/net-and-build

twcclegg commented Aug 3, 2026 •

edited

Loading

Uh oh!

codecov Bot commented Aug 3, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Aug 3, 2026 •

edited

Loading

Uh oh!

wmundev left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

twcclegg commented Aug 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Correctness

Performance

Build and CI

Reviewer notes

Deliberately not done

Uh oh!

codecov Bot commented Aug 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

github-actions Bot commented Aug 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📊 Benchmark Results

Uh oh!

wmundev left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

twcclegg commented Aug 3, 2026 •

edited

Loading

codecov Bot commented Aug 3, 2026 •

edited

Loading

github-actions Bot commented Aug 3, 2026 •

edited

Loading