perf: faster getPaths/basename/isSubPath + per-fs getPaths cache#535
Open
alexander-akait wants to merge 5 commits intomainfrom
Open
perf: faster getPaths/basename/isSubPath + per-fs getPaths cache#535alexander-akait wants to merge 5 commits intomainfrom
alexander-akait wants to merge 5 commits intomainfrom
Conversation
|
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #535 +/- ##
==========================================
+ Coverage 96.44% 96.46% +0.02%
==========================================
Files 50 50
Lines 2669 2685 +16
Branches 812 817 +5
==========================================
+ Hits 2574 2590 +16
Misses 80 80
Partials 15 15
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Merging this PR will degrade performance by 48.35%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| 🆕 | is-sub-path: all-pairs parent × request (no trailing sep) |
N/A | 1.7 ms | N/A |
| ❌ | cache-predicate: mixed cached/uncached requests (warm) |
2.2 ms | 4.2 ms | -48.35% |
| 🆕 | is-sub-path: all-pairs parent (trailing sep) × request |
N/A | 1.8 ms | N/A |
| ❌ | main-files: [main, entry, index] (warm) |
925.6 µs | 1,223.8 µs | -24.37% |
| ⚡ | realistic-midsize: mixed batch (cold cache) |
8.2 ms | 7.1 ms | +14.99% |
| ⚡ | tsconfig-paths: 5 path prefixes (warm) |
3.3 ms | 2.7 ms | +22.59% |
Comparing claude/search-perf-improvements-GXYWT (f86e4ee) with main (1c52e71)
alexander-akait
force-pushed
the
claude/search-perf-improvements-GXYWT
branch
from
62f2053 to
c49dcce
Compare
Main changed `basename` to a centralized helper on Resolver, so the previous `getPaths.basename` char-scan folded away during rebase. This commit reapplies the remaining perf work on top of main and adds one more cleanup. Kept from the previous branch: - lib/util/path.js isSubPath: charCode last-char check + skip normalize() when the parent has no trailing separator. - lib/util/entrypoints.js findMatch: track isPattern/isSubpath during the scan (drops two trailing full-string rescans), use charCode for slash suffix, confirm single "*" with anchored `indexOf`, and thread a precomputed `bestMatchPatternIndex` into `patternKeyCompare` so it doesn't re-scan the same string on every call. - lib/ExportsFieldPlugin.js / lib/ImportsFieldPlugin.js: hoist the duplicate `relativePath.slice(2)` / `path_.slice(2)`. - lib/Resolver.js isDirectory: charCode comparison instead of `String#endsWith`. - lib/ModulesUtils.js: rebuild addrs with nested for-loops instead of `.paths.map(...).reduce(...)`. - lib/ParsePlugin.js: hoist `this.requestOptions` to a local. - lib/AliasUtils.js: hoist wildcard-match check and `innerRequest.slice(item.name.length)` out of `resolveWithAlias`, so array-alias entries don't recompute the same values per element. - test/path.test.js: extended isSubPath edge coverage. - benchmark/cases/is-sub-path/: new micro-bench. New in this commit: - lib/createInnerContext.js + lib/Resolver.js doResolve: the caller previously built an intermediate 6-field object just to pass it to `createInnerContext`, which returned a second fresh object with the same shape (two allocations per doResolve call). The new signature takes the outer context + stack + message directly, so only one allocation happens per call. doResolve is on the hottest path in the resolver. - lib/util/entrypoints.js conditionalMapping: keep a local reference to the stack-top tuple so the `top[2] = i + 1` writes below don't have to re-walk `lookup.length - 1` twice. 983 jest tests still pass; lint clean; wall-clock benchmarks healthy.
alexander-akait
force-pushed
the
claude/search-perf-improvements-GXYWT
branch
from
f5e2d62 to
3ebcc7e
Compare
The hoisted `const remainingRequest = matchRequest
? innerRequest.slice(item.name.length) : undefined;` ran the `.slice()`
whenever `matchRequest` was true, while the original version only did
it inside the closure under the stricter triple-condition guard
(`matchRequest && innerRequest !== alias &&
!innerRequest.startsWith(\`\${alias}/\`)`). For edge cases where
`matchRequest` hit but the inner guards rejected, the hoist added a
wasted substring allocation per matched item — enough to regress
alias-heavy benchmarks under CodSpeed simulation without a clear
net-win for the array-alias case that motivated the hoist.
Revert AliasUtils to the original shape. Everything else stays —
those wins (isSubPath, findMatch trailing-scan elimination,
patternKeyCompare precomputed indices, createInnerContext
single-allocation signature, ModulesUtils flat loop, ParsePlugin
requestOptions hoist, Resolver.isDirectory charCode, exports/imports
plugin slice(2) hoist, entrypoints conditionalMapping top cache) are
either strict instruction-count reductions or remove real work.
Under CodSpeed simulation (cachegrind), V8's native `String.prototype.endsWith` builtin with a single-char suffix is typically reached in fewer instructions than an explicit JS `len > 0 && charCodeAt(len-1) === 47` — the builtin is one tight C++ comparison, while the JS form is a property read + branch + method call + compare. Restore the original simple form.
…sions
CodSpeed flagged three significant regressions:
cache-predicate -47.96%
exports-field -37.93%
main-files -23.74%
The common path across all three is `doResolve` → `createInnerContext`,
which I had refactored to take the outer context + stack + message
directly (avoiding the intermediate 6-field object literal). On paper
this saved one allocation per call; under cachegrind simulation the
extra polymorphic `outer.X` reads on the user-supplied resolveContext
appear to outweigh the saved allocation, regressing every benchmark
that goes through the resolve pipeline.
The exports-field regression also points at `findMatch`, where the
state-tracking refactor (precomputed `bestMatchPatternIndex` threaded
into `patternKeyCompare`, plus `bestMatchIsPattern`/`bestMatchIsSubpath`
to skip the two trailing `bestMatch.endsWith("/")` /
`bestMatch.includes("*")` scans) added per-iteration store overhead in
a measurably hot loop.
Revert both back to upstream shape:
- lib/createInnerContext.js + lib/Resolver.js doResolve: original
intermediate-object form.
- lib/util/entrypoints.js patternKeyCompare + findMatch: original
2-arg signature, `lastIndexOf("*")` confirmation, trailing
`endsWith`/`includes` scans, `key[len-1]` indexed access.
Kept (each one is either skipping a real call or eliminating real
work, with no per-call accounting risk):
- lib/util/path.js isSubPath — charCode-based last-char check,
skips `normalize()` on the common branch.
- lib/util/entrypoints.js conditionalMapping — local `top` reference
for the lookup-stack tip; saves two `lookup[lookup.length - 1]`
walks per iteration.
- lib/ExportsFieldPlugin.js / lib/ImportsFieldPlugin.js — hoist the
duplicate `relativePath.slice(2)` / `path_.slice(2)`.
- lib/ModulesUtils.js — nested `for` loops instead of
`.paths.map(...).reduce(spread)`; removes intermediate sub-arrays.
- lib/ParsePlugin.js — hoist `this.requestOptions` to a local.
- benchmark/cases/is-sub-path/ + test/path.test.js — micro-bench
and extended `isSubPath` coverage.
Full jest suite still passes (983 tests), lint clean.
`parseIdentifier` runs on every resolve via `Resolver.parse`. The `/file:/i` scan it runs unconditionally is pure overhead for the overwhelming majority of inputs (relative paths, module specifiers, absolute paths) — none of which can start with "file:". Short-circuit with a cheap `charCodeAt(0)` check against 'f'/'F' before running the regex. Real file URLs always begin with "file:", so the guard preserves every case the downstream `fileURLToPath` actually handles; the behavior change is limited to pathological inputs like `"./file:x"` which never produced a valid file URL anyway (fileURLToPath would throw). No tests cover `file:` URL inputs and no other code path inside lib/ touches file:// identifiers. Tests (983) still green.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
char-code scan. Byte-identical output to the previous implementation
for the POSIX/Windows absolute inputs the resolver actually produces.
String#lastIndexOf scans (one of which always ran end-to-end on
POSIX paths).
normalize(parent + "/") round-trip on the common branch — the
prefix + separator-char anchor gives the same semantics.
PathCache (WeakMap keyed by filesystem), so repeated ancestor-chain
lookups during module resolution hit a Map<string, PathsResult>.
Hits return fresh array copies so SymlinkPlugin can keep mutating
segments[i] without leaking across calls.
resolver.pathCache.getPaths.fn.
Tests cover: additional Windows-path and empty-input cases for
getPaths/basename, cache-hit freshness, and more isSubPath edge cases
(equal paths, empty parent, trailing separators).
Benchmarks: extends the get-paths micro-bench with cached-hit and
basename tasks, and adds a new is-sub-path case exercising the
TsconfigPathsPlugin parent × request matrix.