fs: improve performance of recursive directory read

Signed-off-by: avivkeller <me@aviv.sh>

Author: avivkeller

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,
@@ -57,9 +56,6 @@ const {
   F_OK,
   O_WRONLY,
   O_SYMLINK,
-  UV_DIRENT_DIR,
-  UV_DIRENT_LINK,
-  UV_DIRENT_UNKNOWN,
 } = constants;
 
 const pathModule = require('path');
@@ -105,8 +101,8 @@ const {
   },
   copyObject,
   Dirent,
-  getDirent,
   getDirents,
+  getRecursiveDirents,
   getOptions,
   getValidatedFd,
   getValidatedPath,
@@ -1742,43 +1738,6 @@ function mkdirSync(path, options) {
   }
 }
 
-/**
- * Appends one directory's entries to `context.results` and the subdirectories
- * still to visit to `context.dirs` (with the prefix their entries get in
- * string results in `context.prefixes`). `result` is a `binding.readdir()`
- * result with file types, so only symbolic links and entries of unknown type
- * need a stat() to find out whether they lead to a directory.
- * @param {string} dir
- * @param {string} prefix
- * @param {[string[], number[]]} result
- * @param {{ withFileTypes: boolean, results: (string | Dirent)[], dirs: string[], prefixes: string[] }} context
- */
-function collectRecursiveReaddirResult(dir, prefix, { 0: names, 1: types }, context) {
-  const { length } = names;
-  for (let i = 0; i < length; i++) {
-    const name = names[i];
-    const relative = prefix === '' ? name : `${prefix}${pathModule.sep}${name}`;
-    let isDirectory;
-    if (context.withFileTypes) {
-      const dirent = getDirent(dir, name, types[i]);
-      ArrayPrototypePush(context.results, dirent);
-      // Follow symbolic links to directories, see https://github.com/nodejs/node/issues/52663
-      isDirectory = dirent.isDirectory() ||
-        (dirent.isSymbolicLink() && binding.internalModuleStat(pathModule.join(dir, name)) === 1);
-    } else {
-      ArrayPrototypePush(context.results, relative);
-      const type = types[i];
-      isDirectory = type === UV_DIRENT_DIR ||
-        ((type === UV_DIRENT_LINK || type === UV_DIRENT_UNKNOWN) &&
-         binding.internalModuleStat(pathModule.join(dir, name)) === 1);
-    }
-    if (isDirectory) {
-      ArrayPrototypePush(context.dirs, pathModule.join(dir, name));
-      ArrayPrototypePush(context.prefixes, relative);
-    }
-  }
-}
-
 /*
  * An recursive algorithm for reading the entire contents of the `basePath` directory.
  * This function does not validate `basePath` as a directory. It is passed directly to
@@ -1792,79 +1751,30 @@ function collectRecursiveReaddirResult(dir, prefix, { 0: names, 1: types }, cont
  * @returns {void}
 */
 function readdirRecursive(basePath, options, callback) {
-  const context = {
-    withFileTypes: Boolean(options.withFileTypes),
-    results: [],
-    dirs: [basePath],
-    prefixes: [''],
+  const withFileTypes = !!options.withFileTypes;
+  const req = new FSReqCallback();
+  req.oncomplete = (err, result) => {
+    if (err) {
+      callback(err);
+      return;
+    }
+    callback(null, withFileTypes ? getRecursiveDirents(basePath, result) : result);
   };
-
-  let i = 0;
-
-  /**
-   * Reads one directory from `context.dirs` and then moves on to the next
-   * one, or calls back once none are left.
-   * @param {string} path
-   * @param {string} prefix path of this directory relative to `basePath`
-   */
-  function read(path, prefix) {
-    const req = new FSReqCallback();
-    req.oncomplete = (err, result) => {
-      if (err) {
-        callback(err);
-        return;
-      }
-
-      if (result === undefined) {
-        callback(null, context.results);
-        return;
-      }
-
-      try {
-        collectRecursiveReaddirResult(path, prefix, result, context);
-      } catch (err) {
-        callback(err);
-        return;
-      }
-
-      if (i < context.dirs.length) {
-        read(context.dirs[i], context.prefixes[i++]);
-      } else {
-        callback(null, context.results);
-      }
-    };
-
-    binding.readdir(path, options.encoding, true, req);
-  }
-
-  read(context.dirs[i], context.prefixes[i++]);
+  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),
-    results: [],
-    dirs: [basePath],
-    prefixes: [''],
-  };
-
-  for (let i = 0; i < context.dirs.length; i++) {
-    const dir = context.dirs[i];
-    const result = binding.readdir(dir, options.encoding, true);
-    if (result !== undefined) {
-      collectRecursiveReaddirResult(dir, context.prefixes[i], result, context);
-    }
-  }
-
-  return context.results;
+  const withFileTypes = !!options.withFileTypes;
+  const result = binding.readdirRecursive(basePath, options.encoding, withFileTypes);
+  return result !== undefined && withFileTypes ? getRecursiveDirents(basePath, result) : result;
 }
 
 /**
@@ -1898,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,
@@ -32,9 +31,6 @@ const {
   O_WRONLY,
   S_IFMT,
   S_IFREG,
-  UV_DIRENT_DIR,
-  UV_DIRENT_LINK,
-  UV_DIRENT_UNKNOWN,
 } = constants;
 
 const binding = internalBinding('fs');
@@ -66,8 +62,8 @@ const {
     kWriteFileMaxChunkSize,
   },
   copyObject,
-  getDirent,
   getDirents,
+  getRecursiveDirents,
   getOptions,
   getStatFsFromBinding,
   getStatsFromBinding,
@@ -1643,41 +1639,17 @@ async function mkdir(path, options) {
 
 async function readdirRecursive(originalPath, options) {
   const withFileTypes = !!options.withFileTypes;
-  const readdirWithTypes = (path) => PromisePrototypeThen(
-    binding.readdir(path, options.encoding, true, kUsePromises),
+  const result = await PromisePrototypeThen(
+    binding.readdirRecursive(
+      originalPath,
+      options.encoding,
+      withFileTypes,
+      kUsePromises,
+    ),
     undefined,
     handleErrorFromBinding,
   );
-  const result = [];
-  const queue = [[originalPath, '', await readdirWithTypes(originalPath)]];
-
-  while (queue.length > 0) {
-    // If we want to implement BFS make this a `shift` call instead of `pop`
-    const { 0: path, 1: prefix, 2: { 0: names, 1: types } } = ArrayPrototypePop(queue);
-    for (let i = 0; i < names.length; i++) {
-      const name = names[i];
-      const relative = prefix === '' ? name : `${prefix}${pathModule.sep}${name}`;
-      let isDirectory;
-      if (withFileTypes) {
-        const dirent = getDirent(path, name, types[i]);
-        ArrayPrototypePush(result, dirent);
-        isDirectory = dirent.isDirectory();
-      } else {
-        ArrayPrototypePush(result, relative);
-        // Entries that are, or may be, symbolic links to directories are followed.
-        const type = types[i];
-        isDirectory = type === UV_DIRENT_DIR ||
-          ((type === UV_DIRENT_LINK || type === UV_DIRENT_UNKNOWN) &&
-           binding.internalModuleStat(pathModule.join(path, name)) === 1);
-      }
-      if (isDirectory) {
-        const direntPath = pathModule.join(path, name);
-        ArrayPrototypePush(queue, [direntPath, relative, await readdirWithTypes(direntPath)]);
-      }
-    }
-  }
-
-  return result;
+  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,