Skip to content

fs: cache glob matcher functions in path.matchesGlob#63915

Open
beeequeue wants to merge 2 commits into
nodejs:mainfrom
beeequeue:matchesGlob-perf
Open

fs: cache glob matcher functions in path.matchesGlob#63915
beeequeue wants to merge 2 commits into
nodejs:mainfrom
beeequeue:matchesGlob-perf

Conversation

@beeequeue

Copy link
Copy Markdown

this PR adds benchmarks for path.matchesGlob() as well as a simple cache for pattern matching functions.

rationale

most glob matching libraries in the ecosystem (including minimatch) provide an API to let users re-use matchers instead of re-creating them every call.

node doesn't provide such an API, and does not re-use matchers when possible, leading to it being much slower (10-15x) than just using new Minimatch() directly when matching a pattern more than once.

implementation details

i decided to "hide" away the cache inside the function rather than add a new API as most users will expect the function to not be a potential footgun, and the pattern has been used before in zeptomatch

since matchGlobPattern() doesn't accept any options other than pattern and path we can safely cache it by the pattern string. if this changes in the future the cache would need to accomodate the new options in the cache keys.

i went with the simplest Map cache implementation i could come up with as i couldn't find any LRU implementations anywhere in the codebase, if you have any suggestions for improvements please let me know!

testing

i'm not sure how to test this properly, e.g. if i should export the cache instance and check its size, etc

some guidance here is very appreciated :)


benchmark results

ran on my windows 11 machine with an AMD Ryzen 9800X3D

note: these benchmarks do not test the case where we evict keys from the cache

                                                                                                                confidence improvement accuracy (*)    (**)   (***)
path\matchesGlob-posix.js n=100000 pattern='src/**/baz/*.test.ts' path='src/foo/bar/biz/baz/index.test.ts'             ***   1013.11 %      ±23.55% ±31.74% ±42.14%
path\matchesGlob-posix.js n=100000 pattern='src/**/baz/*.test.ts' path='src/index.test.ts'                             ***   2005.02 %      ±48.08% ±64.80% ±86.02%
path\matchesGlob-posix.js n=100000 pattern='src/**/baz/*.test.ts' path='src/index.ts'                                  ***   2073.47 %      ±27.53% ±37.10% ±49.24%
path\matchesGlob-posix.js n=100000 pattern='src/**/baz/*.ts' path='src/foo/bar/biz/baz/index.test.ts'                  ***    869.89 %      ±18.72% ±25.22% ±33.48%
path\matchesGlob-posix.js n=100000 pattern='src/**/baz/*.ts' path='src/index.test.ts'                                  ***   1726.85 %      ±29.66% ±39.97% ±53.07%
path\matchesGlob-posix.js n=100000 pattern='src/**/baz/*.ts' path='src/index.ts'                                       ***   1777.43 %      ±25.94% ±34.95% ±46.40%
path\matchesGlob-posix.js n=100000 pattern='test/**/*.ts' path='src/foo/bar/biz/baz/index.test.ts'                     ***   1074.40 %      ±22.04% ±29.70% ±39.43%
path\matchesGlob-posix.js n=100000 pattern='test/**/*.ts' path='src/index.test.ts'                                     ***   1424.64 %      ±28.72% ±38.70% ±51.38%
path\matchesGlob-posix.js n=100000 pattern='test/**/*.ts' path='src/index.ts'                                          ***   1460.18 %      ±17.52% ±23.60% ±31.33%
path\matchesGlob-win32.js n=100000 pattern='src/**/baz/*.test.ts' path='src/foo/bar/biz/baz/index.test.ts'             ***    659.77 %       ±7.00%  ±9.43% ±12.52%
path\matchesGlob-win32.js n=100000 pattern='src/**/baz/*.test.ts' path='src/index.ts'                                  ***   1056.32 %      ±12.77% ±17.21% ±22.84%
path\matchesGlob-win32.js n=100000 pattern='src/**/baz/*.test.ts' path='src\\foo\\bar\\biz\\baz\\index.test.ts'        ***    492.15 %       ±4.26%  ±5.73%  ±7.60%
path\matchesGlob-win32.js n=100000 pattern='src/**/baz/*.test.ts' path='src\\index.ts'                                 ***    858.27 %       ±6.41%  ±8.64% ±11.46%
path\matchesGlob-win32.js n=100000 pattern='src/**/baz/*.ts' path='src/foo/bar/biz/baz/index.test.ts'                  ***    565.03 %       ±5.60%  ±7.54% ±10.01%
path\matchesGlob-win32.js n=100000 pattern='src/**/baz/*.ts' path='src/index.ts'                                       ***    915.51 %       ±9.18% ±12.38% ±16.42%
path\matchesGlob-win32.js n=100000 pattern='src/**/baz/*.ts' path='src\\foo\\bar\\biz\\baz\\index.test.ts'             ***    431.51 %       ±3.11%  ±4.18%  ±5.51%
path\matchesGlob-win32.js n=100000 pattern='src/**/baz/*.ts' path='src\\index.ts'                                      ***    746.71 %       ±6.50%  ±8.75% ±11.62%
path\matchesGlob-win32.js n=100000 pattern='test/**/*.ts' path='src/foo/bar/biz/baz/index.test.ts'                     ***    597.13 %      ±12.85% ±17.31% ±22.97%
path\matchesGlob-win32.js n=100000 pattern='test/**/*.ts' path='src/index.ts'                                          ***    745.44 %      ±12.25% ±16.50% ±21.90%
path\matchesGlob-win32.js n=100000 pattern='test/**/*.ts' path='src\\foo\\bar\\biz\\baz\\index.test.ts'                ***    421.16 %       ±3.44%  ±4.61%  ±6.08%
path\matchesGlob-win32.js n=100000 pattern='test/**/*.ts' path='src\\index.ts'                                         ***    594.88 %      ±10.85% ±14.62% ±19.41%

i also have a personal benchmarking repo where i found this issue, where the results now look like this:

screenshot image

Signed-off-by: Adam Haglund <adam@haglund.dev>
Signed-off-by: Adam Haglund <adam@haglund.dev>
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/performance

@nodejs-github-bot nodejs-github-bot added fs Issues and PRs related to the fs subsystem / file system. needs-ci PRs that need a full CI run. labels Jun 14, 2026
Comment thread lib/internal/fs/glob.js
nocaseMagicOnly: true,
});

patternFnCache ??= new SafeMap();

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'm not sure if it's worth it to delay the map instantiation like this, but did it just in case.

Comment thread lib/internal/fs/glob.js
});
patternFnCache.set(pattern, matcher);

if (patternFnCache.size >= 250) {

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

randomly picked number, i'm not sure how to find a "good" value though.

Comment thread lib/internal/fs/glob.js
if (patternFnCache.has(pattern)) {
matcher = patternFnCache.get(pattern);
} else {
matcher = createMatcher(pattern, {

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i re-used createMatcher from above as i saw that it uses the same default options as the previous lines did.

i thought about putting the caching in that function instead, but then we would need to base the caching on all the options it accepts, so i decided to keep it here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fs Issues and PRs related to the fs subsystem / file system. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants