Skip to content

Commit bb3f917

Browse files
authored
fs: reduce glob section allocations
Store prefix counts in section records to avoid a separate vector and its allocations during multi-globstar matching. Signed-off-by: Roberto Bianchi <roberto.bianchi@spendesk.com> PR-URL: #66716 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day>
1 parent 2052d0d commit bb3f917

4 files changed

Lines changed: 88 additions & 5 deletions

File tree

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
const assert = require('node:assert');
5+
const { posix, win32 } = require('node:path');
6+
7+
const bench = common.createBenchmark(main, {
8+
n: [2e4],
9+
platform: ['posix', 'win32'],
10+
pattern: ['**/node_modules/**/test/**', '**/src/**'],
11+
});
12+
13+
function main({ n, platform, pattern }) {
14+
const implementation = platform === 'posix' ? posix : win32;
15+
const filenames = Array.from({ length: 64 }, (_, index) => {
16+
const directory = index % 2 === 0 ? 'node_modules/dependency/test' : 'src';
17+
return `workspace/packages/package${index}/${directory}/file${index}.js`
18+
.replaceAll('/', implementation.sep);
19+
});
20+
assert.strictEqual(filenames.filter((filename) => implementation.matchesGlob(filename, pattern)).length, 32);
21+
let matches = 0;
22+
bench.start();
23+
for (let iteration = 0; iteration < n; iteration++) {
24+
for (const filename of filenames) matches += implementation.matchesGlob(filename, pattern);
25+
}
26+
bench.end(n * filenames.length);
27+
assert.strictEqual(matches, n * 32);
28+
}
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
const assert = require('node:assert');
5+
const { posix, win32 } = require('node:path');
6+
7+
const bench = common.createBenchmark(main, {
8+
n: [1e5],
9+
platform: ['posix', 'win32'],
10+
count: [1, 2, 4, 10, 16],
11+
result: ['hit', 'miss'],
12+
});
13+
14+
function main({ n, platform, count, result }) {
15+
const implementation = platform === 'posix' ? posix : win32;
16+
const names = Array.from({ length: count - 1 }, (_, index) => `part${index}`);
17+
const pattern = ['**', ...names.flatMap((name) => [name, '**']), 'file.js'].join('/');
18+
const parts = ['root', ...names.flatMap((name) => [name, 'dir']), 'file.js'];
19+
if (result === 'miss') {
20+
if (names.length) parts[parts.length - 3] = 'absent';
21+
else parts[parts.length - 1] = 'file.txt';
22+
}
23+
const filename = parts.join(implementation.sep);
24+
const expected = result === 'hit';
25+
assert.strictEqual(implementation.matchesGlob(filename, pattern), expected);
26+
let actual;
27+
bench.start();
28+
for (let index = 0; index < n; index++) actual = implementation.matchesGlob(filename, pattern);
29+
bench.end(n);
30+
assert.strictEqual(actual, expected);
31+
}

‎src/glob/glob_matcher.cc‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -584,6 +584,7 @@ class PathMatcher {
584584
struct Section {
585585
size_t begin, end; // pattern part indices
586586
ptrdiff_t after; // last possible file index this section can start at
587+
size_t non_gs;
587588
};
588589

589590
Tri MatchGlobstar(Parts file,
@@ -660,13 +661,11 @@ class PathMatcher {
660661
// split body into globstar-delimited sections with their last possible
661662
// start positions
662663
std::vector<Section> sections;
663-
sections.push_back({body_begin, body_begin, 0});
664-
std::vector<size_t> non_gs_sums{0};
664+
sections.push_back({body_begin, body_begin, 0, 0});
665665
size_t non_gs = 0;
666666
for (size_t i = body_begin; i < body_end; i++) {
667667
if (pattern[i].kind == PartMatcher::Kind::kGlobstar) {
668-
non_gs_sums.push_back(non_gs);
669-
sections.push_back({i + 1, i + 1, 0});
668+
sections.push_back({i + 1, i + 1, 0, non_gs});
670669
} else {
671670
sections.back().end = i + 1;
672671
non_gs++;
@@ -677,7 +676,7 @@ class PathMatcher {
677676
{
678677
size_t i = sections.size() - 1;
679678
for (Section& s : sections) {
680-
s.after = file_length - (static_cast<ptrdiff_t>(non_gs_sums[i--]) +
679+
s.after = file_length - (static_cast<ptrdiff_t>(sections[i--].non_gs) +
681680
static_cast<ptrdiff_t>(s.end - s.begin));
682681
}
683682
}
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
'use strict';
2+
3+
require('../common');
4+
const assert = require('node:assert');
5+
const { posix, win32 } = require('node:path');
6+
7+
for (const implementation of [posix, win32]) {
8+
for (const count of [1, 2, 4, 8, 9, 10, 16]) {
9+
for (const prefix of ['part', '\u00e9tage', '\u4e2d']) {
10+
const names = Array.from({ length: count - 1 }, (_, index) => `${prefix}${index}`);
11+
const pattern = ['**', ...names.flatMap((name) => [name, '**']), 'file.js'].join('/');
12+
const parts = ['root', ...names.flatMap((name) => [name, 'dir']), 'file.js'];
13+
assert.strictEqual(implementation.matchesGlob(parts.join(implementation.sep), pattern), true);
14+
if (names.length) {
15+
parts[parts.length - 3] = 'absent';
16+
} else {
17+
parts[parts.length - 1] = 'file.txt';
18+
}
19+
assert.strictEqual(implementation.matchesGlob(parts.join(implementation.sep), pattern), false);
20+
}
21+
}
22+
assert.strictEqual(implementation.matchesGlob('root/.hidden/src/lib/file.js', '**/src/**/file.js'), false);
23+
assert.strictEqual(implementation.matchesGlob('root/src/./lib/file.js', '**/src/**/file.js'), true);
24+
assert.strictEqual(implementation.matchesGlob('root/src/lib/file.js/', '**/src/**/file.js'), true);
25+
}

0 commit comments

Comments
 (0)