fs: improve performance of recursive directory read

benchmark/fs/bench-readdir-recursive.js

@@ -0,0 +1,47 @@
+'use strict';
+
+const common = require('../common');
+const fs = require('fs');
+const path = require('path');
+const assert = require('assert');
+
+const bench = common.createBenchmark(main, {
+  n: [10],
+  dir: ['lib', 'test/parallel'],
+  mode: ['sync', 'callback', 'promise'],
+  withFileTypes: ['true', 'false'],
+});
+
+async function main({ n, dir, mode, withFileTypes }) {
+  withFileTypes = withFileTypes === 'true';
+  const fullPath = path.resolve(__dirname, '../../', dir);
+  const options = { recursive: true, withFileTypes };
+  let entries;
+
+  bench.start();
+  switch (mode) {
+    case 'sync':
+      for (let i = 0; i < n; i++) {
+        entries = fs.readdirSync(fullPath, options);
+      }
+      break;
+    case 'callback':
+      for (let i = 0; i < n; i++) {
+        entries = await new Promise((resolve, reject) => {
+          fs.readdir(fullPath, options, (err, result) => {
+            if (err) reject(err);
+            else resolve(result);
+          });
+        });
+      }
+      break;
+    case 'promise':
+      for (let i = 0; i < n; i++) {
+        entries = await fs.promises.readdir(fullPath, options);
+      }
+      break;
+  }
+  bench.end(n);
+
+  assert.ok(entries.length > 0);
+}

lib/fs.js

@@ -28,7 +28,6 @@ const {
   ArrayFromAsync,
   ArrayPrototypePush,
   BigIntPrototypeToString,
-  Boolean,
   FunctionPrototypeCall,
   MathMax,
   Number,
@@ -102,8 +101,8 @@ const {
   },
   copyObject,
   Dirent,
-  getDirent,
   getDirents,
+  getRecursiveDirents,
   getOptions,
   getValidatedFd,
   getValidatedPath,
@@ -1752,129 +1751,30 @@ function mkdirSync(path, options) {
  * @returns {void}
 */
 function readdirRecursive(basePath, options, callback) {
-  const context = {
-    withFileTypes: Boolean(options.withFileTypes),
-    encoding: options.encoding,
-    basePath,
-    readdirResults: [],
-    pathsQueue: [basePath],
-  };
-
-  let i = 0;
-
-  function read(path) {
-    const req = new FSReqCallback();
-    req.oncomplete = (err, result) => {
-      if (err) {
-        callback(err);
-        return;
-      }
-
-      if (result === undefined) {
-        callback(null, context.readdirResults);
-        return;
-      }
-
-      processReaddirResult({
-        result,
-        currentPath: path,
-        context,
-      });
-
-      if (i < context.pathsQueue.length) {
-        read(context.pathsQueue[i++]);
-      } else {
-        callback(null, context.readdirResults);
-      }
-    };
-
-    binding.readdir(
-      path,
-      context.encoding,
-      context.withFileTypes,
-      req,
-    );
-  }
-
-  read(context.pathsQueue[i++]);
-}
-
-// Calling `readdir` with `withFileTypes=true`, the result is an array of arrays.
-// The first array is the names, and the second array is the types.
-// They are guaranteed to be the same length; hence, setting `length` to the length
-// of the first array within the result.
-const processReaddirResult = (args) => (args.context.withFileTypes ? handleDirents(args) : handleFilePaths(args));
-
-function handleDirents({ result, currentPath, context }) {
-  const { 0: names, 1: types } = result;
-  const { length } = names;
-
-  for (let i = 0; i < length; i++) {
-    // Avoid excluding symlinks, as they are not directories.
-    // Refs: https://github.com/nodejs/node/issues/52663
-    const fullPath = pathModule.join(currentPath, names[i]);
-    const dirent = getDirent(currentPath, names[i], types[i]);
-    ArrayPrototypePush(context.readdirResults, dirent);
-
-    if (dirent.isDirectory() || binding.internalModuleStat(fullPath) === 1) {
-      ArrayPrototypePush(context.pathsQueue, fullPath);
-    }
-  }
-}
-
-function handleFilePaths({ result, currentPath, context }) {
-  for (let i = 0; i < result.length; i++) {
-    const resultPath = pathModule.join(currentPath, result[i]);
-    const relativeResultPath = pathModule.relative(context.basePath, resultPath);
-    const stat = binding.internalModuleStat(resultPath);
-    ArrayPrototypePush(context.readdirResults, relativeResultPath);
-
-    if (stat === 1) {
-      ArrayPrototypePush(context.pathsQueue, resultPath);
+  const withFileTypes = !!options.withFileTypes;
+  const req = new FSReqCallback();
+  req.oncomplete = (err, result) => {
+    if (err) {
+      callback(err);
+      return;
     }
-  }
+    callback(null, withFileTypes ? getRecursiveDirents(basePath, result) : result);
+  };
+  binding.readdirRecursive(basePath, options.encoding, withFileTypes, req);
 }
 
 /**
- * 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`.
- * @param {string} basePath
+ * Synchronously reads the entire contents of the `basePath` directory.
+ * This function does not validate `basePath` as a directory. It is passed
+ * directly to `binding.readdirRecursive`.
+ * @param {string | Buffer} basePath
  * @param {{ encoding: string, withFileTypes: boolean }} options
- * @returns {string[] | Dirent[]}
+ * @returns {string[] | Buffer[] | Dirent[]}
  */
 function readdirSyncRecursive(basePath, options) {
-  const context = {
-    withFileTypes: Boolean(options.withFileTypes),
-    encoding: options.encoding,
-    basePath,
-    readdirResults: [],
-    pathsQueue: [basePath],
-  };
-
-  function read(path) {
-    const readdirResult = binding.readdir(
-      path,
-      context.encoding,
-      context.withFileTypes,
-    );
-
-    if (readdirResult === undefined) {
-      return;
-    }
-
-    processReaddirResult({
-      result: readdirResult,
-      currentPath: path,
-      context,
-    });
-  }
-
-  for (let i = 0; i < context.pathsQueue.length; i++) {
-    read(context.pathsQueue[i]);
-  }
-
-  return context.readdirResults;
+  const withFileTypes = !!options.withFileTypes;
+  const result = binding.readdirRecursive(basePath, options.encoding, withFileTypes);
+  return result !== undefined && withFileTypes ? getRecursiveDirents(basePath, result) : result;
 }
 
 /**
@@ -1908,9 +1808,6 @@ function readdir(path, options, callback) {
   }
 
   if (options.recursive) {
-    // Make shallow copy to prevent mutating options from affecting results
-    options = copyObject(options);
-
     readdirRecursive(path, options, callback);
     return;
   }

lib/internal/fs/promises.js

@@ -1,7 +1,6 @@
 'use strict';
 
 const {
-  ArrayPrototypePop,
   ArrayPrototypePush,
   Error,
   ErrorCaptureStackTrace,
@@ -64,6 +63,7 @@ const {
   },
   copyObject,
   getDirents,
+  getRecursiveDirents,
   getOptions,
   getStatFsFromBinding,
   getStatsFromBinding,
@@ -1638,78 +1638,18 @@ async function mkdir(path, options) {
 }
 
 async function readdirRecursive(originalPath, options) {
-  const result = [];
-  const queue = [
-    [
+  const withFileTypes = !!options.withFileTypes;
+  const result = await PromisePrototypeThen(
+    binding.readdirRecursive(
       originalPath,
-      await PromisePrototypeThen(
-        binding.readdir(
-          originalPath,
-          options.encoding,
-          !!options.withFileTypes,
-          kUsePromises,
-        ),
-        undefined,
-        handleErrorFromBinding,
-      ),
-    ],
-  ];
-
-
-  if (options.withFileTypes) {
-    while (queue.length > 0) {
-      // If we want to implement BFS make this a `shift` call instead of `pop`
-      const { 0: path, 1: readdir } = ArrayPrototypePop(queue);
-      for (const dirent of getDirents(path, readdir)) {
-        ArrayPrototypePush(result, dirent);
-        if (dirent.isDirectory()) {
-          const direntPath = pathModule.join(path, dirent.name);
-          ArrayPrototypePush(queue, [
-            direntPath,
-            await PromisePrototypeThen(
-              binding.readdir(
-                direntPath,
-                options.encoding,
-                true,
-                kUsePromises,
-              ),
-              undefined,
-              handleErrorFromBinding,
-            ),
-          ]);
-        }
-      }
-    }
-  } else {
-    while (queue.length > 0) {
-      const { 0: path, 1: readdir } = ArrayPrototypePop(queue);
-      for (const ent of readdir) {
-        const direntPath = pathModule.join(path, ent);
-        const stat = binding.internalModuleStat(direntPath);
-        ArrayPrototypePush(
-          result,
-          pathModule.relative(originalPath, direntPath),
-        );
-        if (stat === 1) {
-          ArrayPrototypePush(queue, [
-            direntPath,
-            await PromisePrototypeThen(
-              binding.readdir(
-                direntPath,
-                options.encoding,
-                false,
-                kUsePromises,
-              ),
-              undefined,
-              handleErrorFromBinding,
-            ),
-          ]);
-        }
-      }
-    }
-  }
-
-  return result;
+      options.encoding,
+      withFileTypes,
+      kUsePromises,
+    ),
+    undefined,
+    handleErrorFromBinding,
+  );
+  return withFileTypes ? getRecursiveDirents(originalPath, result) : result;
 }
 
 async function readdir(path, options) {

lib/internal/fs/utils.js

@@ -2,6 +2,7 @@
 
 const {
   ArrayIsArray,
+  ArrayPrototypePush,
   BigInt,
   Date,
   DateNow,
@@ -323,6 +324,26 @@ function getDirent(path, name, type, callback) {
   }
 }
 
+/**
+ * Builds the Dirent objects for a recursive readdir
+ * @param {string | Buffer} basePath
+ * @param {[string[] | Buffer[], number[], number[], string[]]} result
+ * @returns {Dirent[]}
+ */
+function getRecursiveDirents(basePath, { 0: names, 1: types, 2: dirIndices, 3: dirs }) {
+  const parentPaths = [basePath];
+  if (dirs.length > 1) {
+    const base = typeof basePath === 'string' ? basePath : `${basePath}`;
+    for (let i = 1; i < dirs.length; i++) {
+      ArrayPrototypePush(parentPaths, pathModule.join(base, dirs[i]));
+    }
+  }
+  for (let i = 0; i < names.length; i++) {
+    names[i] = new Dirent(names[i], types[i], parentPaths[dirIndices[i]]);
+  }
+  return names;
+}
+
 function getOptions(options, defaultOptions = kEmptyObject) {
   if (options == null || typeof options === 'function') {
     return defaultOptions;
@@ -1128,6 +1149,7 @@ module.exports = {
   getDirent,
   getDirents,
   getOptions,
+  getRecursiveDirents,
   getValidatedFd,
   getValidatedPath,
   handleErrorFromBinding,