fs: improve performance of recursive directory read · nodejs/node@2130460 · GitHub
Skip to content

Commit 2130460

Browse files
avivkelleraduh95
authored andcommitted
fs: improve performance of recursive directory read
Fixes: #58892 Refs: #52663 Signed-off-by: avivkeller <me@aviv.sh> PR-URL: #65524 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Shelley Vohr <shelley.vohr@gmail.com> Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
1 parent 6db3f66 commit 2130460

12 files changed

Lines changed: 1024 additions & 134 deletions
Lines changed: 47 additions & 0 deletions

‎benchmark/fs/bench-readdir.js‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,18 +8,16 @@ const bench = common.createBenchmark(main, {
88
n: [10],
99
dir: [ 'lib', 'test/parallel'],
1010
withFileTypes: ['true', 'false'],
11-
recursive: ['true', 'false'],
1211
});
1312

14-
function main({ n, dir, withFileTypes, recursive }) {
13+
function main({ n, dir, withFileTypes }) {
1514
withFileTypes = withFileTypes === 'true';
16-
recursive = recursive === 'true';
1715
const fullPath = path.resolve(__dirname, '../../', dir);
1816
bench.start();
1917
(function r(cntr) {
2018
if (cntr-- <= 0)
2119
return bench.end(n);
22-
fs.readdir(fullPath, { withFileTypes, recursive }, () => {
20+
fs.readdir(fullPath, { withFileTypes }, () => {
2321
r(cntr);
2422
});
2523
}(n));

‎benchmark/fs/bench-readdirSync.js‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,17 +8,15 @@ const bench = common.createBenchmark(main, {
88
n: [10],
99
dir: [ 'lib', 'test/parallel'],
1010
withFileTypes: ['true', 'false'],
11-
recursive: ['true', 'false'],
1211
});
1312

1413

15-
function main({ n, dir, withFileTypes, recursive }) {
14+
function main({ n, dir, withFileTypes }) {
1615
withFileTypes = withFileTypes === 'true';
17-
recursive = recursive === 'true';
1816
const fullPath = path.resolve(__dirname, '../../', dir);
1917
bench.start();
2018
for (let i = 0; i < n; i++) {
21-
fs.readdirSync(fullPath, { withFileTypes, recursive });
19+
fs.readdirSync(fullPath, { withFileTypes });
2220
}
2321
bench.end(n);
2422
}

‎lib/fs.js‎

Lines changed: 47 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ const {
2828
ArrayFromAsync,
2929
ArrayPrototypePush,
3030
BigIntPrototypeToString,
31-
Boolean,
3231
FunctionPrototypeCall,
3332
MathMax,
3433
Number,
@@ -57,9 +56,6 @@ const {
5756
F_OK,
5857
O_WRONLY,
5958
O_SYMLINK,
60-
UV_DIRENT_DIR,
61-
UV_DIRENT_LINK,
62-
UV_DIRENT_UNKNOWN,
6359
} = constants;
6460

6561
const pathModule = require('path');
@@ -103,10 +99,11 @@ const {
10399
kReadFileBufferLength,
104100
kMaxUserId,
105101
},
102+
collectRecursiveReaddirResult,
106103
copyObject,
107104
Dirent,
108-
getDirent,
109105
getDirents,
106+
getRecursiveDirents,
110107
getOptions,
111108
getValidatedFd,
112109
getValidatedPath,
@@ -1744,57 +1741,40 @@ function mkdirSync(path, options) {
17441741
}
17451742

17461743
/**
1747-
* Appends one directory's entries to `context.results` and the subdirectories
1748-
* still to visit to `context.dirs` (with the prefix their entries get in
1749-
* string results in `context.prefixes`). `result` is a `binding.readdir()`
1750-
* result with file types, so only symbolic links and entries of unknown type
1751-
* need a stat() to find out whether they lead to a directory.
1752-
* @param {string} dir
1753-
* @param {string} prefix
1754-
* @param {[string[], number[]]} result
1755-
* @param {{ withFileTypes: boolean, results: (string | Dirent)[], dirs: string[], prefixes: string[] }} context
1756-
*/
1757-
function collectRecursiveReaddirResult(dir, prefix, { 0: names, 1: types }, context) {
1758-
const { length } = names;
1759-
for (let i = 0; i < length; i++) {
1760-
const name = names[i];
1761-
const relative = prefix === '' ? name : `${prefix}${pathModule.sep}${name}`;
1762-
let isDirectory;
1763-
if (context.withFileTypes) {
1764-
const dirent = getDirent(dir, name, types[i]);
1765-
ArrayPrototypePush(context.results, dirent);
1766-
// Follow symbolic links to directories, see https://github.com/nodejs/node/issues/52663
1767-
isDirectory = dirent.isDirectory() ||
1768-
(dirent.isSymbolicLink() && binding.internalModuleStat(pathModule.join(dir, name)) === 1);
1769-
} else {
1770-
ArrayPrototypePush(context.results, relative);
1771-
const type = types[i];
1772-
isDirectory = type === UV_DIRENT_DIR ||
1773-
((type === UV_DIRENT_LINK || type === UV_DIRENT_UNKNOWN) &&
1774-
binding.internalModuleStat(pathModule.join(dir, name)) === 1);
1775-
}
1776-
if (isDirectory) {
1777-
ArrayPrototypePush(context.dirs, pathModule.join(dir, name));
1778-
ArrayPrototypePush(context.prefixes, relative);
1779-
}
1780-
}
1781-
}
1782-
1783-
/*
1784-
* An recursive algorithm for reading the entire contents of the `basePath` directory.
1785-
* This function does not validate `basePath` as a directory. It is passed directly to
1786-
* `binding.readdir`.
1787-
* @param {string} basePath
1744+
* Reads the entire contents of the `basePath` directory. This function does
1745+
* not validate `basePath` as a directory. It is passed directly to
1746+
* `binding.readdirRecursive`, or to `binding.readdir` for each directory when
1747+
* the permission model is enabled, since that checks every directory on the
1748+
* main thread where the native walk can not.
1749+
* @param {string | Buffer} basePath
17881750
* @param {{ encoding: string, withFileTypes: boolean }} options
17891751
* @param {(
17901752
* err?: Error,
17911753
* files?: string[] | Buffer[] | Dirent[]
17921754
* ) => any} callback
17931755
* @returns {void}
1794-
*/
1756+
*/
17951757
function readdirRecursive(basePath, options, callback) {
1758+
if (permission.isEnabled()) {
1759+
readdirRecursiveWithPermissionModel(basePath, options, callback);
1760+
return;
1761+
}
1762+
const withFileTypes = !!options.withFileTypes;
1763+
const req = new FSReqCallback();
1764+
req.oncomplete = (err, result) => {
1765+
if (err) {
1766+
callback(err);
1767+
return;
1768+
}
1769+
callback(null, withFileTypes ? getRecursiveDirents(basePath, result) : result);
1770+
};
1771+
binding.readdirRecursive(basePath, options.encoding, withFileTypes, req);
1772+
}
1773+
1774+
function readdirRecursiveWithPermissionModel(basePath, options, callback) {
1775+
const { encoding } = options;
17961776
const context = {
1797-
withFileTypes: Boolean(options.withFileTypes),
1777+
withFileTypes: !!options.withFileTypes,
17981778
results: [],
17991779
dirs: [basePath],
18001780
prefixes: [''],
@@ -1816,11 +1796,6 @@ function readdirRecursive(basePath, options, callback) {
18161796
return;
18171797
}
18181798

1819-
if (result === undefined) {
1820-
callback(null, context.results);
1821-
return;
1822-
}
1823-
18241799
try {
18251800
collectRecursiveReaddirResult(path, prefix, result, context);
18261801
} catch (err) {
@@ -1835,37 +1810,37 @@ function readdirRecursive(basePath, options, callback) {
18351810
}
18361811
};
18371812

1838-
binding.readdir(path, options.encoding, true, req);
1813+
binding.readdir(path, encoding, true, req);
18391814
}
18401815

18411816
read(context.dirs[i], context.prefixes[i++]);
18421817
}
18431818

18441819
/**
1845-
* An iterative algorithm for reading the entire contents of the `basePath` directory.
1846-
* This function does not validate `basePath` as a directory. It is passed directly to
1847-
* `binding.readdir`.
1848-
* @param {string} basePath
1820+
* Synchronously reads the entire contents of the `basePath` directory, see
1821+
* `readdirRecursive`.
1822+
* @param {string | Buffer} basePath
18491823
* @param {{ encoding: string, withFileTypes: boolean }} options
1850-
* @returns {string[] | Dirent[]}
1824+
* @returns {string[] | Buffer[] | Dirent[]}
18511825
*/
18521826
function readdirSyncRecursive(basePath, options) {
1853-
const context = {
1854-
withFileTypes: Boolean(options.withFileTypes),
1855-
results: [],
1856-
dirs: [basePath],
1857-
prefixes: [''],
1858-
};
1859-
1860-
for (let i = 0; i < context.dirs.length; i++) {
1861-
const dir = context.dirs[i];
1862-
const result = binding.readdir(dir, options.encoding, true);
1863-
if (result !== undefined) {
1827+
const withFileTypes = !!options.withFileTypes;
1828+
if (permission.isEnabled()) {
1829+
const context = {
1830+
withFileTypes,
1831+
results: [],
1832+
dirs: [basePath],
1833+
prefixes: [''],
1834+
};
1835+
for (let i = 0; i < context.dirs.length; i++) {
1836+
const dir = context.dirs[i];
1837+
const result = binding.readdir(dir, options.encoding, true);
18641838
collectRecursiveReaddirResult(dir, context.prefixes[i], result, context);
18651839
}
1840+
return context.results;
18661841
}
1867-
1868-
return context.results;
1842+
const result = binding.readdirRecursive(basePath, options.encoding, withFileTypes);
1843+
return result !== undefined && withFileTypes ? getRecursiveDirents(basePath, result) : result;
18691844
}
18701845

18711846
/**
@@ -1899,9 +1874,6 @@ function readdir(path, options, callback) {
18991874
}
19001875

19011876
if (options.recursive) {
1902-
// Make shallow copy to prevent mutating options from affecting results
1903-
options = copyObject(options);
1904-
19051877
readdirRecursive(path, options, callback);
19061878
return;
19071879
}

‎lib/internal/fs/promises.js‎

Lines changed: 33 additions & 36 deletions

0 commit comments

Comments
 (0)