perf: match files with a compiled picomatch and walk with tinyglobby - #316
Merged
Conversation
`globby` walked the file system and `micromatch` matched a path against the patterns — two packages for two jobs, but between them they carried `fast-glob`, `braces` and a second `picomatch`, and our `globby` was four majors behind the one `stylelint` already ships. `tinyglobby` walks with `fdir`, and `picomatch` is the matcher `micromatch` wraps, so neither is a new name in the tree. The production dependency tree drops from 66 packages to 44, and walking the file system is measurably faster (0.29 ms against 0.55 ms over the stylelint fixtures, best of five). The larger win is not the package count. `micromatch.isMatch(file, patterns)` compiles the patterns on every call, and the plugin calls it per module per check from `succeedModule`. Compiling once in `resolveCheck`, where the patterns are built, costs 0.0001 ms against 0.0043 ms a call. `dot` is kept on both, so a hidden directory reached by a wildcard is still linted; Node's own `fs.globSync` and `path.matchesGlob` were measured first and are both slower than what they would replace, and neither takes the option. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GzZci4NQeiqwdrVfd7dGXy
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
`normalize-path` was called on one value, the result of `path.resolve`, which has already collapsed the duplicate separators and the trailing slash it exists to remove. What is left of it there is the separator swap, which is a `replaceAll`. It is also wrong twice on the shapes `resolve` returns on Windows, and both were measured before the swap: `\\server\share\a.css` came back as `/server/share/a.css`, no longer naming the share, and the drive root `C:\` came back as `C:`, which names whatever the drive's working directory is. Both keep their shape now, and a unit test pins all four shapes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GzZci4NQeiqwdrVfd7dGXy
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.
Summary
globbywalked the file system,micromatchmatched a path against the patterns, andnormalize-pathturned the separators around — three packages, three jobs, not duplicates. But between them they carriedfast-glob,bracesand a secondpicomatch, and ourglobbywas four majors behind the onestylelintalready ships.tinyglobbywalks withfdir,picomatchis the matchermicromatchwraps, and the separator swap is areplaceAll— so nothing new enters the tree and one package leaves it entirely. The production dependency tree drops from 66 packages to 43.The larger win is not the package count.
micromatch.isMatch(file, patterns, options)compiles the patterns on every call, and the plugin calls it per module per check fromsucceedModule. Compiling once inresolveCheck, where the patterns are already built, is 43× cheaper per call:micromatch.isMatch0.0043 mspicomatch0.0001 msglobby.sync0.55 mstinyglobby.globSync0.29 msBest of five over the
test/stylelint/fixturestree; the walk runs once per build, the match once per module per check.Node's own built-ins were measured first and rejected. At our
>= 22.12floor bothfs.globSyncandpath.matchesGlobexist and would cost zero dependencies, but they are slower than what they would replace — 1.18 ms and 0.0223 ms against the numbers above — and neither takes adotoption, so a hidden directory reached by a wildcard would silently stop being linted.tinyglobbyandpicomatchboth take it, so that behaviour is unchanged.normalize-pathwas called on one value: the result ofpath.resolve, which has already collapsed the duplicate separators and the trailing slash that package exists to remove. What was left of it there is the separator swap. It was also wrong twice on the shapesresolvereturns on Windows, and both were measured before the swap:normalize-path\\server\share\a.css/server/share/a.css— no longer the share//server/share/a.cssC:\C:— drive-relative, not the rootC:/A unit test pins all four shapes.
Output was compared against
globbybefore the swap over the plugin's real pattern shapes — a directory, a bare file path, several patterns at once, one matching nothing, and a symlinked directory — and is identical in every case.What kind of change does this PR introduce?
perf.
Did you add tests for your changes?
Yes, a
toPosixPathcase intest/utils.test.jscovering the POSIX, Windows, UNC and drive-root shapes. The glob swap has identical output and is covered by the existing suite —exclude,ignore,context,resource-query,lint-dirty-modules-onlyand bothstylelintfile-walking suites all exercise those paths. 122 passing.Does this PR introduce a breaking change?
No.
dotis preserved, the matched and walked file sets were verified identical, and the two path shapes that change were previously resolved to a different location than the one named.If relevant, what needs to be documented once your changes are merged or what have you already documented?
n/a — no user-facing option changes. The changeset is a patch.
Use of AI
Written with Claude Code, driven interactively. I asked whether two glob packages were really needed, pointed it at
tinyglobby, and then asked fornormalize-pathto go too. It measured rather than assumed: it benchmarkedtinyglobby, Node's built-infs.globSync/path.matchesGloband a hand-rolledreaddirwalk against the current code, found the built-ins slower and lackingdot, and found on the way that the per-module matcher was recompiling its patterns on every call — which is the bigger part of this change. It diffed the file sets over the plugin's real pattern shapes before swapping, and diffednormalize-pathagainst a plain separator swap over every shapepath.resolvecan return, which is how the two wrong ones turned up. I reviewed the result.🤖 Generated with Claude Code
https://claude.ai/code/session_01GzZci4NQeiqwdrVfd7dGXy