Skip to content

fix: improve range parsing performance - #918

Open
reggi wants to merge 1 commit into
mainfrom
reggi/improve-range-parsing-performance
Open

fix: improve range parsing performance#918
reggi wants to merge 1 commit into
mainfrom
reggi/improve-range-parsing-performance

Conversation

@reggi

@reggi reggi commented Oct 8, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

  • Replace comparator whitespace normalization with a regex-free scanner that preserves legacy match boundaries.
  • Cache numeric lookahead to avoid repeatedly scanning overlapping digit runs.
  • Share the ECMAScript whitespace definition between production and tests, using a byte lookup table rather than a Set.
  • Add legacy compatibility checks and large-prefix/numeric-input regression coverage.

Stack

Originally stacked on #917, which was squash-merged into main as 7257c14. Rebased onto main after that merge so this PR contains only the comparator normalization implementation and its tests. The implementation patch is unchanged.

Validation

  • Full npm test passed: 54 test files, 100% coverage, ESLint, and template checks.
  • Differential review: 302,520 scanner cases and 100,000 Range cases matched the parent implementation.
  • Whitespace lookup: 196,608 differential cases matched the original scanner across all UTF-16 code units.
  • Large invalid-prefix parsing improved dramatically; numeric-lookahead access counts scaled linearly through one million characters.

Performance tradeoff

Local Node v26 benchmarks showed approximately 6–7% slower ordinary uncached Range parsing versus the original regex. The shared byte-table lookup did not introduce the additional slowdown observed with Set-based membership checks. Measurements are local microbenchmarks, not cross-runtime guarantees.

@reggi
reggi requested a review from a team as a code owner October 8, 2026 15:08
This was referenced Oct 8, 2026
Merged
Closed
reggi added a commit that referenced this pull request Oct 16, 2026
@reggi
fix: avoid comparator trim regex in v7 range parsing
b598ea8 
Use the regex-free forward scanner from PR #918 while retaining the existing regex exports.

Include regression coverage for compatibility, exported regex preservation, and parsing without COMPARATORTRIM execution.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 03b4a44c-26a3-4d56-9b26-cd46006844c6
@reggi
reggi force-pushed the reggi/improve-range-parsing-performance branch from 66d057c to b598ea8 Compare October 16, 2026 19:33
@reggi
reggi force-pushed the reggi/chore-adds-tests branch from 6e8d7d1 to 0fc683a Compare October 16, 2026 19:33
This was referenced Oct 16, 2026
Merged
Open
Merged
Open
nishantms
nishantms previously approved these changes Oct 16, 2026
View reviewed changes
martinrrm
martinrrm previously approved these changes Oct 19, 2026
View reviewed changes
nkolev92
nkolev92 previously approved these changes Oct 19, 2026
View reviewed changes
Comment thread internal/trim-comparator-whitespace.js
whitespace[char.charCodeAt(0)] = 1
}

/**

@nkolev92 nkolev92 Oct 19, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: The full description on helpers like isWhitespace, isDigit and a few others feel heavier than the implementation.
Just something to consider.

Comment thread internal/trim-comparator-whitespace.js
}

/**
* Reads a number using the old regex's length and leading-zero rules.

@nkolev92 nkolev92 Oct 19, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a temporal comment, so it only makes sense in the context of the PR, but probably needs more detail to stand independently.

reggi added a commit that referenced this pull request Oct 20, 2026
@reggi
chore: cover comparator normalization (#917)
7257c14 
## Summary
- Add regression fixtures and integration coverage for comparator
whitespace, equality boundaries, and malformed loose alternatives.
- Cover long prerelease identifiers, build-metadata normalization, and
existing version limits across strict/loose and prerelease options.
- Tests only; no production behavior changes.

## Stack
First of two stacked PRs; followed by #918. Contains only commit
1eda6b4. Merge this PR before the
performance implementation.

## Validation
Full `npm test` (including coverage, ESLint, and template checks) passes
on the combined stack.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 03b4a44c-26a3-4d56-9b26-cd46006844c6
Base automatically changed from reggi/chore-adds-tests to main October 20, 2026 17:07
@reggi
reggi dismissed stale reviews from nkolev92, martinrrm, and nishantms October 20, 2026 17:07

The base branch was changed.

reggi added a commit that referenced this pull request Oct 20, 2026
@reggi
chore: backport v7 range parsing coverage to v5 (#926)
b827b90 
## Summary
Backport the v7 range parsing coverage from #917 to v5. Add coverage for
comparator whitespace normalization, range parsing options, equality
boundaries, malformed loose alternatives, long identifiers, and build
metadata.

Use the same range fixture inputs and options as v7 while preserving
v5-specific expected results. Tests only; no production code or
dependency changes.

## Stack
First of two stacked v5 backport PRs, based on `release/v5`. Contains
test commit `8bbf475`. The follow-up backport of the v7 performance fix
in #918 targets this branch. Merge this PR before the v5 fix.

The follow-up v5 fix is #927.

## Validation
The full v5 test suite passed locally on Node 20.19.5 with 100%
statement, branch, function, and line coverage; one test was skipped.
The full supported Node-version matrix remains unverified locally.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 03b4a44c-26a3-4d56-9b26-cd46006844c6
reggi added a commit that referenced this pull request Oct 20, 2026
@reggi
chore: backport v7 range parsing coverage to v6 (#924)
9a3f125 
## Summary
Backport the v7 range parsing coverage from #917 to v6. Add coverage for
comparator whitespace normalization, range parsing options, equality
boundaries, malformed loose alternatives, long identifiers, and build
metadata.

Use the same range fixture inputs and options as v7 while preserving
v6-specific expected results. Tests only; no production code or
dependency changes.

## Stack
First of two stacked v6 backport PRs, based on `release/v6`. Contains
test commit `b381d7f`. The follow-up backport of the v7 performance fix
in #918 targets this branch. Merge this PR before the v6 fix.

The follow-up v6 fix is #925.

## Validation
The full v6 test suite passed locally on Node 20.19.5 with 100%
statement, branch, function, and line coverage; one test was skipped.
The full supported Node-version matrix remains unverified locally.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 03b4a44c-26a3-4d56-9b26-cd46006844c6
@reggi
fix: avoid comparator trim regex in v7 range parsing
1b3a949 
Use the regex-free forward scanner from PR #918 while retaining the existing regex exports.

Include regression coverage for compatibility, exported regex preservation, and parsing without COMPARATORTRIM execution.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 03b4a44c-26a3-4d56-9b26-cd46006844c6
@reggi
reggi force-pushed the reggi/improve-range-parsing-performance branch from b598ea8 to 1b3a949 Compare October 21, 2026 19:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nkolev92 nkolev92 nkolev92 left review comments
@martinrrm martinrrm martinrrm left review comments
@nishantms nishantms nishantms left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@reggi @nkolev92 @martinrrm @nishantms