Skip to content

refactor(router): compare tail and prefix segments in place - #199

Merged
dinwwwh merged 1 commit into
mainfrom
claude/project-thread-m32ra9
Oct 29, 2026
Merged

dinwwwh merged 1 commit into
mainfrom
claude/project-thread-m32ra9

Conversation

@dinwwwh

@dinwwwh dinwwwh commented Oct 29, 2026

Copy link
Copy Markdown
Member

Requested by dinwwwh · project thread

What

matchesSegment was a one-line function used in two places: matchesCatchAll, for the segments after a catch-all param, and the closure that createPrefixMatcher returns. Both now do the comparison in place, with a comment that undefined stands for a param, which needs a non-empty segment.

Why

Each top-level function costs V8 about 15k instructions to set up at import, and about as much again to compile when it is first called (see #198). This removes one function from every import. It also removes one compile from the first request that reaches a catch-all, or a prefix with a param.

Measurements

Instruction counts from callgrind, node --single-threaded, averaged over hash seeds 1 and 2:

before after
min / min+gzip 5043 / 2100 B 5053 / 2098 B
import 2.167M 2.164M
REST cold+10 (includes a catch-all request) 6.041M 6.017M (−0.4%)
steady, catch-all / tail 4985 / 5256 4979 / 5249
steady, prefix matcher with a param / with % 2453 / 4082 2422 / 4052 (−1%)

Cold+3 on the REST set reaches no catch-all, so it changes only by the import saving.

Checks

  • Tests pass with 100% coverage.
  • The model fuzzers, including the late-add and prefix ones, find 0 diffs.
  • Stryker finds 68 survivors, the same as on main. None are on the new comparisons.

Generated by Claude Code

`matchesSegment` was a one-line function shared by `matchesCatchAll` and
the prefix matcher. Comparing in place spares V8 a function to set up at
import and to compile on the first request that reaches a catch-all or a
prefix with a param.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VVaw7Sutbw6xMTMyDFeNfw
@dinwwwh dinwwwh self-assigned this Oct 29, 2026
@codecov

codecov Bot commented Oct 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@codspeed

codspeed Bot commented Oct 29, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

1 warning

⚠️ 1 benchmark is dominated by system calls

System calls cannot be simulated consistently and are left out of the measure. The reported value understates the real cost. Switch to the Walltime instrument to measure them.

Learn more about system calls

See affected benchmarks

✅ 29 untouched benchmarks
⏩ 2 skipped benchmarks1


Comparing claude/project-thread-m32ra9 (74625ac) with main (cd598d9)

Open in CodSpeed

Footnotes

  1. 2 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ No new issues found.

Reviewed changes

  • matchesSegment inlined at both call sites — the module-private expected === undefined ? segment !== '' : segment === expected helper is removed and its negation written directly in createPrefixMatcher's closure and in matchesCatchAll's tail loop. The inlined ternaries are the exact De Morgan negation of the helper, so behavior is unchanged; I confirmed both sites and ran the router suite (73 tests pass).
  • Comment added documenting that undefined stands for a param requiring a non-empty segment.

The refactor is mechanical and provably behavior-preserving: !matchesSegment expands to expected === undefined ? segment === '' : segment !== expected on both branches. matchesSegment was module-private with only these two callers (impact scan found no tracked references), and no public export changed.

Pullfrog  | View workflow run | Using deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

@dinwwwh
dinwwwh merged commit 55cb4e1 into main Oct 29, 2026
11 checks passed
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.

2 participants