Skip to content

refactor(router): lower the shared-parts count once per braced part - #201

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

expand tracks how many of the previous route's parts the next route can reuse (literals). It used to lower that count in two places: once where a route waits, and once where it adds a param. Both happen only for a part that starts with { once decoded, so the count is now lowered in one place, right after that check.

That check also catches a part such as {a.b}, which starts with { but is literal text. The count now drops for that part too, so the next route shares fewer parts with this one and parses them again. Matching is unaffected. The comment on shared and literals now describes the rule this way.

Measurements

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

before after
min / min+gzip 5012 / 2092 B 5001 / 2091 B
REST cold+3 match 3.498M 3.485M (−0.35%)
REST cold+3, one request per method 5.075M 5.064M (−0.2%)
REST cold+10 6.010M 5.998M (−0.2%)

expand only runs while the tree is being built, so steady-state matching does not change.

Checks


Generated by Claude Code

`expand` lowered `literals` in both the wait branch and the param branch.
Doing it once for every part starting with `{` covers both, and a literal
part starting with `{` only shares fewer parts with the next route.

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 (fa5ae66) with main (f18cd06)

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

  • Single-site literals lowering — if (index < literals) { literals = index } moved from the wait branch and the param branch to one spot immediately after if (segment[0] === '{'), so the shared-parts count drops once per braced part.
  • Comment update — the shared/literals comment now describes the rule as "the parts before the first part starting with { once decoded", which matches the code.
  • Behavioral delta — a literal part that starts with { after decoding (e.g. {a.b}) now lowers literals too. This is the one semantic change the PR calls out, and I verified it is safe: new literals is always ≤ old (lowering happens at a superset of indices), the count resets per route, and a smaller count only stops the next route's shared-prefix walk earlier, re-walking idempotent literalChild/child.param ??= steps. The extra expand(child.pending, …) those re-walks can trigger is a no-op, since the route that set literals already flushed that region. A differential fuzz over {a.b} / %7Ba.b%7D permutations with late-add warmups (forcing whole === true and partial builds) found 0 mismatches, and late-adds.test.ts:49 already covers the duplicate {a.b} case.

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

@dinwwwh
dinwwwh merged commit 8e11ec5 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