perf: speed up ConfigArray config resolution - #498
Conversation
Speeds up getConfigWithStatus() and isDirectoryIgnored() by roughly 4x on posix paths and 1.7x on Windows paths: * Compile minimatch patterns to regular expressions via makeRe() and cache them, instead of calling Minimatch#match(), which re-splits the file path on every call. Negation, trailing-slash, and trailing- globstar semantics of match() are preserved exactly, with a fallback to match() for patterns that can't be compiled safely. * Add a fast path to toRelativePath() for already-normalized absolute posix paths, avoiding resolve()/relative() and their allocations. * Cache derived per-config metadata (universal files partition and global-ignores detection) in a WeakMap instead of recomputing it for every file path. * Replace reduce()/some()/filter() callbacks and temporary wrapper objects in the matching hot paths with allocation-free loops. * Skip building debug message strings when debug logging is disabled. Also adds a benchmark script (npm run bench in packages/config-array) for measuring config resolution performance. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
One or more co-authors of this pull request were not found. You must specify co-authors in commit message trailer via: Supported
Alternatively, if the co-author should not be included, remove the Please update your commit message(s) by doing |
There was a problem hiding this comment.
Pull request overview
Optimizes ConfigArray path matching and config resolution while adding performance benchmarking support.
Changes:
- Compiles and caches glob matchers and config metadata.
- Adds optimized POSIX path handling and allocation-reduced matching loops.
- Adds a configurable config-resolution benchmark and documentation.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
packages/config-array/src/config-array.js |
Optimizes matching, path resolution, caching, and debug handling. |
packages/config-array/package.json |
Adds the benchmark command. |
packages/config-array/benchmarks/README.md |
Documents benchmark usage and interpretation. |
packages/config-array/benchmarks/config-resolution.bench.js |
Implements the benchmark workload and reporting. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
|
||
| const useWindowsPaths = process.argv.includes("--windows"); | ||
| const runsArg = process.argv.find(arg => arg.startsWith("--runs=")); | ||
| const RUNS = runsArg ? Number(runsArg.slice("--runs=".length)) : 100; |
There was a problem hiding this comment.
Fixed in 232818f. --runs now goes through a parseRuns() helper that rejects anything that isn't a positive integer:
$ node benchmarks/config-resolution.bench.js --runs=0
Error: Invalid --runs value "0": expected a positive integer.
Same for abc, 2.5, and -5, so the fractional-value mismatch you noted is covered too.
| let regexp; | ||
| const hasTrailingGlobstar = rawMatcher.set.some( | ||
| alternative => | ||
| alternative.length > 1 && alternative.at(-1) === GLOBSTAR, | ||
| ); | ||
|
|
||
| if (!hasTrailingGlobstar) { | ||
| regexp = rawMatcher.makeRe() || null; | ||
| } else { | ||
| regexp = null; | ||
|
|
||
| if (rawMatcher.set.length === 1) { | ||
| const compiled = rawMatcher.makeRe(); | ||
|
|
||
| if (compiled && compiled.source.endsWith(")?$")) { | ||
| regexp = new RegExp( | ||
| `${compiled.source.slice(0, -2)}$`, | ||
| compiled.flags, | ||
| ); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Good catch, and investigating it turned up something worse than the recursion bound: makeRe() is lossy for these patterns, not just unbounded.
For a pattern with more than one globstar after the first path segment, makeRe() silently drops all but the first one. src/**/test/**/*.js compiled to a regex equivalent to src/**/test/*.js, so src/test/x/a.js stopped matching — a silent behavior regression, not just a performance/safety question. My original differential fuzzing missed it because the pattern corpus never had two internal globstars.
Fixed in 232818f by gating the fast path on canCompileToRegExp(): patterns with more than one non-leading globstar stay on match(), which resolves your maxGlobstarRecursion point as well, since those are exactly the patterns the bound applies to. Patterns with a leading globstar plus at most one later globstar compile correctly and still take the fast path, so **/*.js, src/**, **/node_modules/**, and packages/*/src/** are unaffected and the speedup is unchanged (posix 15.4ms → 3.3ms median, interleaved A/B).
Also added regression tests for multi-globstar patterns; 3 of the 4 fail without the gate. Verified with an exhaustive differential run against the pre-change implementation: 1,554 generated patterns × 680 paths = 1.06M comparisons, zero mismatches.
On backtracking for the patterns that still compile: I measured match() vs RegExp#test() on 40KB adversarial paths (20k segments). The regex stays flat at well under 1ms, while match() degrades to ~250-280ms for **/node_modules/** and **/a/**. So for the shapes remaining on the fast path, the regex is the better-behaved of the two on hostile input.
Addresses review feedback on the config resolution optimization. `Minimatch#makeRe()` silently drops all but the first globstar that appears after the first path segment, so a pattern such as `src/<globstar>/test/<globstar>/*.js` compiled to a regular expression that no longer matched `src/test/x/a.js`. Patterns like that now stay on `Minimatch#match()`, which also keeps them subject to minimatch's `maxGlobstarRecursion` bound. Patterns with a leading globstar and at most one later globstar still compile, which covers the common cases, so the performance gain is unchanged. Adds regression tests for multi-globstar patterns, which the existing suite did not cover, and validates the benchmark's `--runs` option so that non-integer or non-positive values fail with a clear error instead of throwing later on undefined.
|
/easycla |
|
Hi everyone, it looks like we lost track of this pull request. Please review and see what the next steps are. This pull request will auto-close in 7 days without an update. |
What
Speeds up
getConfigWithStatus()andisDirectoryIgnored()in@eslint/config-arrayby roughly 4x on posix paths and 1.7x on Windows paths, and adds a benchmark script for measuring config resolution performance.Benchmark (1,500 unique file paths against an 8-config array, 100 runs, same machine, mean time per run):
Changes
doMatch()now compiles each pattern once viaMinimatch#makeRe()and evaluatesregexp.test()instead ofMinimatch#match(), which re-splits the file path and walks the pattern segment-by-segment on every call. Three semantic differences betweenmatch()and the compiled regex are compensated exactly:!-stripped pattern), so normal andflipNegatematching share one code path;"foo/"matches pattern"foo");"a/**"must not match"a"), with a fallback tomatch()for patterns that can't be rewritten safely (e.g. brace expansions with a trailing globstar).toRelativePath()short-circuits for already-normalized absolute posix paths: aslice()when the path is under the base, or a ported allocation-lightrelative()walk when it isn't. Windows paths keep the original code path.filespartition and the global-ignores key check are computed once per config object (WeakMap) instead of per file x per config.reduce()/some()/filter()callbacks, per-call regex literal, and temporary{ files, ignores }wrapper objects inshouldIgnorePath()/pathMatches()are replaced with plain indexed loops (matchesIgnores(),pathMatchesFiles()).debug.enabledis true.npm run benchinpackages/config-array(options:--windows,--runs=N); seebenchmarks/README.md.Verification
..segments, which are normalized away or rejected as external before matching.@jsr/std__pathacross 200k random path pairs: zero mismatches.basePathconfigs, negated patterns, brace patterns, and function matchers, on both posix and Windows base paths): zero mismatches in status, merged config, or directory-ignore results.🤖 Generated with Claude Code