Skip to content

Commit

Permalink
fs: make readdir recursive algorithm iterative
Browse files Browse the repository at this point in the history
PR-URL: #47650
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
  • Loading branch information
Ethan-Arrowood authored and MoLow committed Jul 6, 2023
1 parent 2cc8715 commit 9b44c56
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 32 deletions.
88 changes: 58 additions & 30 deletions lib/fs.js
Expand Up @@ -27,6 +27,7 @@
const {
ArrayPrototypePush,
BigIntPrototypeToString,
Boolean,
MathMax,
Number,
ObjectDefineProperties,
Expand Down Expand Up @@ -95,6 +96,7 @@ const {
copyObject,
Dirent,
emitRecursiveRmdirWarning,
getDirent,
getDirents,
getOptions,
getValidatedFd,
Expand Down Expand Up @@ -1399,34 +1401,60 @@ function mkdirSync(path, options) {
}
}

// TODO(Ethan-Arrowood): Make this iterative too
function readdirSyncRecursive(path, origPath, options) {
nullCheck(path, 'path', true);
const ctx = { path };
const result = binding.readdir(pathModule.toNamespacedPath(path),
options.encoding, !!options.withFileTypes, undefined, ctx);
handleErrorFromBinding(ctx);
return options.withFileTypes ?
getDirents(path, result).flatMap((dirent) => {
return [
dirent,
...(dirent.isDirectory() ?
readdirSyncRecursive(
pathModule.join(path, dirent.name),
origPath,
options,
) : []),
];
}) :
result.flatMap((ent) => {
const innerPath = pathModule.join(path, ent);
const relativePath = pathModule.relative(origPath, innerPath);
const stat = binding.internalModuleStat(innerPath);
return [
relativePath,
...(stat === 1 ? readdirSyncRecursive(innerPath, origPath, options) : []),
];
});
/**
* An iterative algorithm for reading the entire contents of the `basePath` directory.
* This function does not validate `basePath` as a directory. It is passed directly to
* `binding.readdir` after a `nullCheck`.
* @param {string} basePath
* @param {{ encoding: string, withFileTypes: boolean }} options
* @returns {string[] | Dirent[]}
*/
function readdirSyncRecursive(basePath, options) {
nullCheck(basePath, 'path', true);

const withFileTypes = Boolean(options.withFileTypes);
const encoding = options.encoding;

const readdirResults = [];
const pathsQueue = [basePath];

const ctx = { path: basePath };
function read(path) {
ctx.path = path;
const readdirResult = binding.readdir(
pathModule.toNamespacedPath(path),
encoding,
withFileTypes,
undefined,
ctx,
);
handleErrorFromBinding(ctx);

for (let i = 0; i < readdirResult.length; i++) {
if (withFileTypes) {
const dirent = getDirent(path, readdirResult[0][i], readdirResult[1][i]);
ArrayPrototypePush(readdirResults, dirent);
if (dirent.isDirectory()) {
ArrayPrototypePush(pathsQueue, pathModule.join(dirent.path, dirent.name));
}
} else {
const resultPath = pathModule.join(path, readdirResult[i]);
const relativeResultPath = pathModule.relative(basePath, resultPath);
const stat = binding.internalModuleStat(resultPath);
ArrayPrototypePush(readdirResults, relativeResultPath);
// 1 indicates directory
if (stat === 1) {
ArrayPrototypePush(pathsQueue, resultPath);
}
}
}
}

for (let i = 0; i < pathsQueue.length; i++) {
read(pathsQueue[i]);
}

return readdirResults;
}

/**
Expand All @@ -1451,7 +1479,7 @@ function readdir(path, options, callback) {
}

if (options.recursive) {
callback(null, readdirSyncRecursive(path, path, options));
callback(null, readdirSyncRecursive(path, options));
return;
}

Expand Down Expand Up @@ -1489,7 +1517,7 @@ function readdirSync(path, options) {
}

if (options.recursive) {
return readdirSyncRecursive(path, path, options);
return readdirSyncRecursive(path, options);
}

const ctx = { path };
Expand Down
2 changes: 0 additions & 2 deletions lib/internal/fs/dir.js
Expand Up @@ -160,8 +160,6 @@ class Dir {
}
}

// TODO(Ethan-Arrowood): Review this implementation. Make it iterative.
// Can we better leverage the `kDirOperationQueue`?
readSyncRecursive(dirent) {
const ctx = { path: dirent.path };
const handle = dirBinding.opendir(
Expand Down

0 comments on commit 9b44c56

Please sign in to comment.