Skip to content

perf: remove linked-vigesimal parser allocations - #1920

Merged
clairernovotny merged 1 commit into
mainfrom
codex/codeql-linked-vigesimal
Aug 1, 2026
Merged

perf: remove linked-vigesimal parser allocations#1920
clairernovotny merged 1 commit into
mainfrom
codex/codeql-linked-vigesimal

Conversation

@clairernovotny

Copy link
Copy Markdown
Member

Summary

  • replace two captured LINQ scans over immutable profile arrays with direct indexed loops
  • preserve longest-first matching, scale ordering, bounds, and short-circuit behavior
  • remove stable per-parse iterator/delegate allocations

Resolves CodeQL library alerts #557 and #558.

Validation

  • owning LinkedVigesimal/Welsh/Yoruba tests: 183/183 on net8.0, net10.0, and net11.0
  • full Humanizer.Tests: 61,016/61,016 on net8.0, net10.0, and net11.0
  • public-path A/B on net8/net10/net11 preserved exact outputs and reduced allocations:
    • Yoruba boundary: 3,808 to 1,800 B/op (-52.73%)
    • Yoruba composite: 1,304 to 744 B/op (-42.94%)
    • Welsh exact phrase: 592 to 440 B/op (-25.68%)
  • net48 Release build: 0 warnings, 0 errors
  • dotnet format Humanizer.slnx --verify-no-changes --verbosity diagnostic: 0/2,821 files changed
  • docs manifest/current/historical validation passed
  • docs runtime, snapshot, trimmed, and native-AOT validation passed
  • Release pack succeeded
  • tests/verify-packages.ps1 passed
  • independent read-only allocation and code reviews: approved with no blocking findings

Checklist

  • Implementation is clean
  • Code adheres to the existing coding standards
  • No Code Analysis warnings
  • Existing owning tests cover the behavior-preserving loop rewrite
  • No code was copied from StackOverflow, a blog, or OSS
  • No comments or public APIs changed
  • Based on live main at branch creation (5cb5e3d987ea3e7ace96dba9b56e73db8ba6e5bf)
  • Later main change fix(generator): filter invalid canonical entries #1917 touches only source-generator tests/docs; the affected source blob is unchanged
  • CodeQL alert links are included above; there is no corresponding issue
  • Readme changes are not required because behavior and public API are unchanged
  • Applicable validation from AGENTS.md was run

@coderabbitai

coderabbitai Bot commented Aug 1, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 677dc08e-22eb-4e9d-a796-f67dbe0faf13

📥 Commits

Reviewing files that changed from the base of the PR and between e997755 and fc189ef.

📒 Files selected for processing (1)
  • src/Humanizer/Localisation/WordsToNumber/LinkedVigesimalWordsToNumberConverter.cs

📝 Walkthrough

Summary by CodeRabbit

  • Refactor
    • Improved internal number-word parsing iteration without changing matching logic or user-visible behavior.

Walkthrough

The converter replaces two LINQ-based iterations with indexed for loops. Scale matching and exact-word parsing behavior remain unchanged.

Changes

Words-to-number parsing

Layer / File(s) Summary
Indexed converter loops
src/Humanizer/Localisation/WordsToNumber/LinkedVigesimalWordsToNumberConverter.cs
FindScaleCountEnd and TryParseExact now iterate through profile arrays by index. Matching conditions remain unchanged.

Estimated code review effort: 1 (Trivial) | ~3 minutes

Poem

A rabbit loops through words with care,
By index it hops from here to there.
Scales still match, exact words stay true,
Fewer LINQ trails to travel through.
Small code changes, neatly done—
Parsing keeps its steady run.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: removing allocations from the linked-vigesimal parser.
Description check ✅ Passed The description directly explains the loop rewrite, allocation reduction, addressed alerts, and validation results.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/codeql-linked-vigesimal

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@clairernovotny
clairernovotny merged commit 4ee66d4 into main Aug 1, 2026
17 checks passed
@clairernovotny
clairernovotny deleted the codex/codeql-linked-vigesimal branch August 1, 2026 01:28
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