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', 'test'],
+  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);
+}

benchmark/fs/bench-readdir.js

@@ -8,18 +8,16 @@ const bench = common.createBenchmark(main, {
   n: [10],
   dir: [ 'lib', 'test/parallel'],
   withFileTypes: ['true', 'false'],
-  recursive: ['true', 'false'],
 });
 
-function main({ n, dir, withFileTypes, recursive }) {
+function main({ n, dir, withFileTypes }) {
   withFileTypes = withFileTypes === 'true';
-  recursive = recursive === 'true';
   const fullPath = path.resolve(__dirname, '../../', dir);
   bench.start();
   (function r(cntr) {
     if (cntr-- <= 0)
       return bench.end(n);
-    fs.readdir(fullPath, { withFileTypes, recursive }, () => {
+    fs.readdir(fullPath, { withFileTypes }, () => {
       r(cntr);
     });
   }(n));

benchmark/fs/bench-readdirSync.js

@@ -8,17 +8,15 @@ const bench = common.createBenchmark(main, {
   n: [10],
   dir: [ 'lib', 'test/parallel'],
   withFileTypes: ['true', 'false'],
-  recursive: ['true', 'false'],
 });
 
 
-function main({ n, dir, withFileTypes, recursive }) {
+function main({ n, dir, withFileTypes }) {
   withFileTypes = withFileTypes === 'true';
-  recursive = recursive === 'true';
   const fullPath = path.resolve(__dirname, '../../', dir);
   bench.start();
   for (let i = 0; i < n; i++) {
-    fs.readdirSync(fullPath, { withFileTypes, recursive });
+    fs.readdirSync(fullPath, { withFileTypes });
   }
   bench.end(n);
 }

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');
@@ -103,10 +99,11 @@ const {
     kReadFileBufferLength,
     kMaxUserId,
   },
+  collectRecursiveReaddirResult,
   copyObject,
   Dirent,
-  getDirent,
   getDirents,
+  getRecursiveDirents,
   getOptions,
   getValidatedFd,
   getValidatedPath,
@@ -1743,57 +1740,40 @@ 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
- * `binding.readdir`.
- * @param {string} basePath
+ * Reads the entire contents of the `basePath` directory. This function does
+ * not validate `basePath` as a directory. It is passed directly to
+ * `binding.readdirRecursive`, or to `binding.readdir` for each directory when
+ * the permission model is enabled, since that checks every directory on the
+ * main thread where the native walk can not.
+ * @param {string | Buffer} basePath
  * @param {{ encoding: string, withFileTypes: boolean }} options
  * @param {(
  *   err?: Error,
  *   files?: string[] | Buffer[] | Dirent[]
  * ) => any} callback
  * @returns {void}
-*/
+ */
 function readdirRecursive(basePath, options, callback) {
+  if (permission.isEnabled()) {
+    readdirRecursiveWithPermissionModel(basePath, options, callback);
+    return;
+  }
+  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);
+}
+
+function readdirRecursiveWithPermissionModel(basePath, options, callback) {
+  const { encoding } = options;
   const context = {
-    withFileTypes: Boolean(options.withFileTypes),
+    withFileTypes: !!options.withFileTypes,
     results: [],
     dirs: [basePath],
     prefixes: [''],
@@ -1815,11 +1795,6 @@ function readdirRecursive(basePath, options, callback) {
         return;
       }
 
-      if (result === undefined) {
-        callback(null, context.results);
-        return;
-      }
-
       try {
         collectRecursiveReaddirResult(path, prefix, result, context);
       } catch (err) {
@@ -1834,37 +1809,37 @@ function readdirRecursive(basePath, options, callback) {
       }
     };
 
-    binding.readdir(path, options.encoding, true, req);
+    binding.readdir(path, encoding, true, req);
   }
 
   read(context.dirs[i], context.prefixes[i++]);
 }
 
 /**
- * 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, see
+ * `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) {
+  const withFileTypes = !!options.withFileTypes;
+  if (permission.isEnabled()) {
+    const context = {
+      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);
       collectRecursiveReaddirResult(dir, context.prefixes[i], result, context);
     }
+    return context.results;
   }
-
-  return context.results;
+  const result = binding.readdirRecursive(basePath, options.encoding, withFileTypes);
+  return result !== undefined && withFileTypes ? getRecursiveDirents(basePath, result) : result;
 }
 
 /**
@@ -1898,9 +1873,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');
@@ -65,9 +61,10 @@ const {
     kReadFileUnknownBufferLength,
     kWriteFileMaxChunkSize,
   },
+  collectRecursiveReaddirResult,
   copyObject,
-  getDirent,
   getDirents,
+  getRecursiveDirents,
   getOptions,
   getStatFsFromBinding,
   getStatsFromBinding,
@@ -1648,42 +1645,42 @@ async function mkdir(path, options) {
 }
 
 async function readdirRecursive(originalPath, options) {
+  if (permission.isEnabled()) {
+    return readdirRecursiveWithPermissionModel(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 withFileTypes ? getRecursiveDirents(originalPath, result) : result;
+}
 
-  return result;
+// TODO: native?
+async function readdirRecursiveWithPermissionModel(basePath, options) {
+  const { encoding } = options;
+  const context = {
+    withFileTypes: !!options.withFileTypes,
+    results: [],
+    dirs: [basePath],
+    prefixes: [''],
+  };
+  for (let i = 0; i < context.dirs.length; i++) {
+    const dir = context.dirs[i];
+    const result = await PromisePrototypeThen(
+      binding.readdir(dir, encoding, true, kUsePromises),
+      undefined,
+      handleErrorFromBinding,
+    );
+    collectRecursiveReaddirResult(dir, context.prefixes[i], result, context);
+  }
+  return context.results;
 }
 
 async function readdir(path, options) {