perf: match static patterns without picomatch - #212
Open
webpro wants to merge 2 commits into
Open
Conversation
commit: |
Owner
|
i will need some time to review this pull request, but i want to tell you in advance that this is an insane improvement, thanks a lot for taking your time to look into this ❤️ |
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
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.
Every walked entry is tested against every compiled pattern. That is fine for globs, but a static pattern can only ever match one exact path, so running it through picomatch costs
O(entries × patterns)for no benefit.This surfaces when patterns are generated rather than hand-written. In knip for example,
package.json#exports/importsmaps expand into entry paths; in a large monorepo with generated-types workspaces a single map can expand to tens of thousands of literal paths (webpro-nl/knip#1876).So I took a stab at this, since I believe it's tinyglobby's job rather than consumer to optimize this. And it will benefit other consumers as well.
The idea is that
processPatternsnow splits positive patterns intostaticandmatch(dynamic). The crawler matches statics with twoSets instead of picomatch:When both kinds are present the matchers are OR'd. When there are no static patterns the original matchers are used untouched, so "pure glob input" is unaffected.
A false negative here only costs speed, while a false positive would change results. So anything outside that set (glob syntax, escapes, quotes,
|,$$,++, non-ascii) keeps the matcher path. It also keeps no picomatch-specific knowledge, so it stays correct if the matcher is ever swapped (looking at #178).Benchmarks
npm run bench— this adds two high-cardinality cases to the existing suite:Worth noting tinyglobby was behind fast-glob and glob on this input before, and is ahead after.
It also helps mixed input, in proportion to how many literals are in the mix (4000-file tree,
**/*plus N literals):Pure-literal input scales linearly instead of quadratically: 1000 literals 81 ms → 4.7 ms, 5000 literals 2308 ms → 21 ms, and the new path stays linear (~4 µs/pattern) out to 50k.
Correctness
No behaviour change — this only changes which matcher a pattern is routed to.
Adds 30 tests covering the static path: missing files, overlap with dynamic patterns, crawler ordering, case sensitivity,
deep, abort signals,ignore,onlyDirectories, absolute patterns and absolute input, symlinks andfollowSymbolicLinks, escaped/quoted/repeated matcher syntax, dotted paths, parent and sibling directories, thefsoption, root hoisting with many patterns, prefix collisions, and pipe alternation.Results were also diffed against
mainover an adversarial corpus (names built from$ ^ + " ' @ ! % ~ # & , ; |, spaces, unicode, nested and space-containing directories) and over randomized inputs across the option matrix (dot,caseSensitiveMatch,onlyFiles,onlyDirectories,deep,ignore,absolute,expandDirectories) — identical in every case.Disclaimer: I've used Claude and Codex to assist, such as for writing extensive tests. This PR is likely "a bit much", kept it together to ensure it does improve performance and is otherwise complete and correct. Happy to tune it down any way you like.