fs: make readdir recursive algorithm iterative

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>
nodejs · May 10, 2023 · 12a93ce · 12a93ce
1 parent ded6b61
commit 12a93ce
Show file tree

Hide file tree

Showing 2 changed files with 58 additions and 32 deletions.
diff --git a/lib/fs.js b/lib/fs.js
@@ -27,6 +27,7 @@
 const {
   ArrayPrototypePush,
   BigIntPrototypeToString,
+  Boolean,
   MathMax,
   Number,
   ObjectDefineProperties,
@@ -97,6 +98,7 @@ const {
   copyObject,
   Dirent,
   emitRecursiveRmdirWarning,
+  getDirent,
   getDirents,
   getOptions,
   getValidatedFd,
@@ -1404,34 +1406,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;
 }
 
 /**
@@ -1456,7 +1484,7 @@ function readdir(path, options, callback) {
   }
 
   if (options.recursive) {
-    callback(null, readdirSyncRecursive(path, path, options));
+    callback(null, readdirSyncRecursive(path, options));
     return;
   }
 
@@ -1494,7 +1522,7 @@ function readdirSync(path, options) {
   }
 
   if (options.recursive) {
-    return readdirSyncRecursive(path, path, options);
+    return readdirSyncRecursive(path, options);
   }
 
   const ctx = { path };

diff --git a/lib/internal/fs/dir.js b/lib/internal/fs/dir.js
@@ -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(