Skip to content

perf(test): add first-use-per-region cold-start benchmark - #438

Merged
twcclegg merged 2 commits into
mainfrom
perf/regression-coverage
Aug 31, 2026
Merged

perf(test): add first-use-per-region cold-start benchmark#438
twcclegg merged 2 commits into
mainfrom
perf/regression-coverage

Conversation

@twcclegg

@twcclegg twcclegg commented Aug 29, 2026

Copy link
Copy Markdown
Owner

Summary

  • Adds four ColdStartBenchmark.FirstUse* benchmarks, each doing a real end-to-end call against a fresh instance (or, for FirstUseAsYouType, a fresh formatter off the shared warm PhoneNumberUtil) targeting one region from a fixed, disjoint pool that GlobalSetup never pre-warms — one previously-unseen region per invocation:
    • FirstUseValidateAndFormatParse + IsValidNumber + Format (20 regions)
    • FirstUseAsYouTypeAsYouTypeFormatter.InputDigit, keystroke by keystroke (15 regions)
    • FirstUseFindNumbersPhoneNumberUtil.FindNumbers (15 regions)
    • FirstUseGeocodePhoneNumberOfflineGeocoder.GetDescriptionForNumber, via the internal constructor so each invocation gets an un-warmed prefix-map cache (15 regions)
  • All four pools are disjoint from each other and from every region GlobalSetup touches — necessary because every benchmark class in this project runs inside one process, and PhoneRegex's pattern cache (and the geocoder's prefix-map cache) are process-wide, not per-instance.
  • Everything currently in this project is untouched — this is additive only. The existing ColdStartBenchmark benchmarks (metadata-loading only) and the other classes' steady-state benchmarks (diverse regions, but pre-warmed in setup) both stay exactly as they are; they're good and valid for what they measure.
  • Documents why this specific benchmark shape exists in README.md, including the two real regressions (#161's fix for the 2017 issue, and #325's 2026 cold-start tradeoff) that this exact gap let ship unnoticed, plus the disjoint-pool convention so a future benchmark addition doesn't accidentally re-warm a region another FirstUse* benchmark depends on staying cold.

Why this specific gap

Every existing benchmark here either repeats one region (cheap after the first call) or warms its whole diverse region set in GlobalSetup before the timed run starts — e.g. PhoneNumberWorkflowBenchmark's seed-data generation calls GetExampleNumberForType/IsValidNumber/Format against every supported region while building its dataset, and AsYouTypeFormatterBenchmark/PhoneNumberMatcherBenchmark/PhoneNumberOfflineGeocoderBenchmark do the equivalent for their own subsystems. None of these shapes can see the cost of a region's genuinely first use in the process. PhoneRegex's pattern cache (PhoneRegex.cs) is a static ConcurrentDictionary, process-wide — a fresh instance doesn't reset it, only a never-before-touched region does. The geocoder's prefix-map cache (PrefixFileReader.cs) is architecturally different (lazy-loaded maps, no regex) but has the identical structural blind spot in its own benchmark class, so it's included for symmetry/completeness across the library's region-keyed subsystems.

Locally, all four FirstUse* benchmarks show multi-millisecond medians versus FirstRegionLookup's (metadata-load-only) ~300us — cleanly separating the first-touch regex-compile/lazy-load cost this class was missing from the metadata-load cost it already measured.

CI wiring — already fully automatic, no changes needed

  • Fails the build on a real regression, in any benchmark, including these new ones. run_performance_tests.yml runs --filter "*" for both the branch and the PR base, lib/compare-benchmarks.js diffs every matching case (Welch's t-test + a 20% relative-delta floor, chosen well above measured single-launch noise), and lib/fail-on-benchmark-regression.js exits 1 — failing the workflow — if significant-changes.json has any regressions entries, regardless of which specific case regressed.
  • Posts a PR comment on a real improvement, in any benchmark, including these new ones. post_performance_test_comment.yml (a separate workflow_run-triggered job) posts/updates a "📊 Statistically significant benchmark improvement" comment with a markdown table whenever significant-changes.json has any improvements entries, and removes the comment if a later push has none.
  • Both operate generically over whatever cases show up in the JSON reports — no allowlist, no per-benchmark configuration. The four new FirstUse* cases are picked up automatically.

Test plan

  • dotnet build csharp — whole solution builds clean
  • dotnet test csharp/PhoneNumbers.slnx -p:TargetFrameworks=net10.0 — 451/451 pass, unaffected (PerformanceTest-only change)
  • dotnet run -c Release --framework net10.0 -- --filter "*ColdStartBenchmark*" locally — all seven benchmarks run cleanly, no exceptions/NA, each FirstUse* benchmark shows the expected cold-vs-warm separation

Every existing benchmark here either repeats one region (cheap after
the first call) or warms its whole diverse region set in GlobalSetup
before the timed run starts (PhoneNumberWorkflowBenchmark's seed-data
generation calls GetExampleNumberForType/IsValidNumber/Format against
every supported region). Neither shape can see the cost of a region's
genuinely first use in the process, which is exactly where two real
regressions lived: the 2017 RegexOptions.Compiled + undersized
RegexCache issue (~115x, fixed by PR #161) and the 2026 PR #325
Compiled-regex change (~100x cold-start cost per new region, not
caught by its own benchmark because that benchmark's setup already
pre-warms every region it measures).

Add ColdStartBenchmark.FirstUseValidateAndFormat: fresh
PhoneNumberUtil per iteration, a fixed 20-region list GlobalSetup
never touches, one previously-unseen region's full
Parse+IsValidNumber+Format per invocation. Locally: 19.6ms median vs
FirstRegionLookup's 300us (metadata-load-only), cleanly separating
the regex-compile cost from the metadata-load cost this class already
measured.

No wiring changes needed - run_performance_tests.yml already runs
`--filter "*"` for both branch and base and diffs every case via
lib/compare-benchmarks.js's Welch's-t-test + 20%-floor comparison, so
the new case is covered automatically. Documented the two-regression
history and the "why" for this specific benchmark shape in README.md
so a future benchmark addition doesn't accidentally drop the property
that makes this one work.
@codecov

codecov Bot commented Aug 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.26%. Comparing base (0bf7478) to head (0584e65).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #438   +/-   ##
=======================================
  Coverage   87.26%   87.26%           
=======================================
  Files          41       41           
  Lines        3831     3831           
  Branches      978      978           
=======================================
  Hits         3343     3343           
  Misses        284      284           
  Partials      204      204           

☔ 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.

FirstUseValidateAndFormat only covered the Parse/IsValidNumber/Format
path. AsYouTypeFormatter and PhoneNumberMatcher share the same
PhoneRegex pattern cache and are exposed to the identical class of
regression; PhoneNumberOfflineGeocoder has an analogous lazy-load blind
spot in its own benchmark class. Add FirstUseAsYouType,
FirstUseFindNumbers, and FirstUseGeocode, each with its own disjoint
15-region pool so no benchmark in the process pre-warms another's
regions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant