perf(node/fs): speedup fs.readdir + async

- Avoid shape transition when `withFileTypes` option was used
- Add fast path for usages without `withFileTypes` to skip allocations

cli/standalone/file_system.rs

@@ -270,6 +270,21 @@ impl FileSystem for DenoCompileFileSystem {
     }
   }
 
+  fn read_dir_names_sync(&self, path: &Path) -> FsResult<Vec<String>> {
+    if self.0.is_path_within(path) {
+      Ok(self.0.read_dir_names(path)?)
+    } else {
+      RealFs.read_dir_names_sync(path)
+    }
+  }
+  async fn read_dir_names_async(&self, path: PathBuf) -> FsResult<Vec<String>> {
+    if self.0.is_path_within(&path) {
+      Ok(self.0.read_dir_names(&path)?)
+    } else {
+      RealFs.read_dir_names_async(path).await
+    }
+  }
+
   fn rename_sync(&self, oldpath: &Path, newpath: &Path) -> FsResult<()> {
     self.error_if_in_vfs(oldpath)?;
     self.error_if_in_vfs(newpath)?;

cli/standalone/virtual_fs.rs

@@ -796,6 +796,17 @@ impl FileBackedVfs {
     )
   }
 
+  pub fn read_dir_names(&self, path: &Path) -> std::io::Result<Vec<String>> {
+    let dir = self.dir_entry(path)?;
+    Ok(
+      dir
+        .entries
+        .iter()
+        .map(|entry| entry.name().to_string())
+        .collect(),
+    )
+  }
+
   pub fn read_link(&self, path: &Path) -> std::io::Result<PathBuf> {
     let (_, entry) = self.fs_root.find_entry_no_follow(path)?;
     match entry {

cli/tools/test/fmt.rs

@@ -326,6 +326,7 @@ pub const OP_DETAILS: phf::Map<&'static str, [&'static str; 2]> = phf_map! {
   "op_fs_mkdir_async" => ["create a directory", "awaiting the result of a `Deno.mkdir` call"],
   "op_fs_open_async" => ["open a file", "awaiting the result of a `Deno.open` call"],
   "op_fs_read_dir_async" => ["read a directory", "collecting all items in the async iterable returned from a `Deno.readDir` call"],
+  "op_fs_read_dir_names_async" => ["read a directory", "collecting all paths as a string array"],
   "op_fs_read_file_async" => ["read a file", "awaiting the result of a `Deno.readFile` call"],
   "op_fs_read_file_text_async" => ["read a text file", "awaiting the result of a `Deno.readTextFile` call"],
   "op_fs_read_link_async" => ["read a symlink", "awaiting the result of a `Deno.readLink` call"],
@@ -397,7 +398,7 @@ mod tests {
 
   // https://github.com/denoland/deno/issues/13729
   // https://github.com/denoland/deno/issues/13938
-  leak_format_test!(op_unknown, true, [RuntimeActivity::AsyncOp(0, None, "op_unknown")], 
+  leak_format_test!(op_unknown, true, [RuntimeActivity::AsyncOp(0, None, "op_unknown")],
     " - An async call to op_unknown was started in this test, but never completed.\n\
     To get more details where leaks occurred, run again with the --trace-leaks flag.\n");
 }

ext/fs/in_memory_fs.rs

@@ -294,6 +294,13 @@ impl FileSystem for InMemoryFs {
     self.read_dir_sync(&path)
   }
 
+  fn read_dir_names_sync(&self, _path: &Path) -> FsResult<Vec<String>> {
+    Err(FsError::NotSupported)
+  }
+  async fn read_dir_names_async(&self, path: PathBuf) -> FsResult<Vec<String>> {
+    self.read_dir_names_sync(&path)
+  }
+
   fn rename_sync(&self, _oldpath: &Path, _newpath: &Path) -> FsResult<()> {
     Err(FsError::NotSupported)
   }

ext/fs/interface.rs

@@ -184,6 +184,9 @@ pub trait FileSystem: std::fmt::Debug + MaybeSend + MaybeSync {
   fn read_dir_sync(&self, path: &Path) -> FsResult<Vec<FsDirEntry>>;
   async fn read_dir_async(&self, path: PathBuf) -> FsResult<Vec<FsDirEntry>>;
 
+  fn read_dir_names_sync(&self, path: &Path) -> FsResult<Vec<String>>;
+  async fn read_dir_names_async(&self, path: PathBuf) -> FsResult<Vec<String>>;
+
   fn rename_sync(&self, oldpath: &Path, newpath: &Path) -> FsResult<()>;
   async fn rename_async(
     &self,

ext/fs/lib.rs

@@ -204,6 +204,8 @@ deno_core::extension!(deno_fs,
     op_fs_realpath_sync<P>,
     op_fs_realpath_async<P>,
     op_fs_read_dir_sync<P>,
+    op_fs_read_dir_names_async<P>,
+    op_fs_read_dir_names_sync<P>,
     op_fs_read_dir_async<P>,
     op_fs_rename_sync<P>,
     op_fs_rename_async<P>,

ext/fs/ops.rs

@@ -573,6 +573,29 @@ where
   Ok(entries)
 }
 
+#[op2]
+#[serde]
+pub fn op_fs_read_dir_names_sync<P>(
+  state: &mut OpState,
+  #[string] path: String,
+) -> Result<Vec<String>, AnyError>
+where
+  P: FsPermissions + 'static,
+{
+  let path = PathBuf::from(path);
+
+  state
+    .borrow_mut::<P>()
+    .check_read(&path, "Deno.readDirSync()")?;
+
+  let fs = state.borrow::<FileSystemRc>();
+  let entries = fs
+    .read_dir_names_sync(&path)
+    .context_path("readdir", &path)?;
+
+  Ok(entries)
+}
+
 #[op2(async)]
 #[serde]
 pub async fn op_fs_read_dir_async<P>(
@@ -600,6 +623,33 @@ where
   Ok(entries)
 }
 
+#[op2(async)]
+#[serde]
+pub async fn op_fs_read_dir_names_async<P>(
+  state: Rc<RefCell<OpState>>,
+  #[string] path: String,
+) -> Result<Vec<String>, AnyError>
+where
+  P: FsPermissions + 'static,
+{
+  let path = PathBuf::from(path);
+
+  let fs = {
+    let mut state = state.borrow_mut();
+    state
+      .borrow_mut::<P>()
+      .check_read(&path, "Deno.readDir()")?;
+    state.borrow::<FileSystemRc>().clone()
+  };
+
+  let entries = fs
+    .read_dir_names_async(path.clone())
+    .await
+    .context_path("readdir", &path)?;
+
+  Ok(entries)
+}
+
 #[op2(fast)]
 pub fn op_fs_rename_sync<P>(
   state: &mut OpState,

ext/fs/std_fs.rs

@@ -187,6 +187,13 @@ impl FileSystem for RealFs {
     spawn_blocking(move || read_dir(&path)).await?
   }
 
+  fn read_dir_names_sync(&self, path: &Path) -> FsResult<Vec<String>> {
+    read_dir_names(path)
+  }
+  async fn read_dir_names_async(&self, path: PathBuf) -> FsResult<Vec<String>> {
+    spawn_blocking(move || read_dir_names(&path)).await?
+  }
+
   fn rename_sync(&self, oldpath: &Path, newpath: &Path) -> FsResult<()> {
     fs::rename(oldpath, newpath).map_err(Into::into)
   }
@@ -868,6 +875,18 @@ fn read_dir(path: &Path) -> FsResult<Vec<FsDirEntry>> {
   Ok(entries)
 }
 
+fn read_dir_names(path: &Path) -> FsResult<Vec<String>> {
+  let entries = fs::read_dir(path)?
+    .filter_map(|entry| {
+      let entry = entry.ok()?;
+      let name = entry.file_name().into_string().ok()?;
+      Some(name)
+    })
+    .collect();
+
+  Ok(entries)
+}
+
 #[cfg(not(windows))]
 fn symlink(
   oldpath: &Path,

ext/node/polyfills/_fs/_fs_dir.ts

@@ -47,12 +47,20 @@ export default class Dir {
         AsyncGeneratorPrototypeNext(this.#asyncIterator),
         (iteratorResult) => {
           resolve(
-            iteratorResult.done ? null : new Dirent(iteratorResult.value),
+            iteratorResult.done ? null : new Dirent(
+              iteratorResult.value.name,
+              this.path,
+              iteratorResult.value,
+            ),
           );
           if (callback) {
             callback(
               null,
-              iteratorResult.done ? null : new Dirent(iteratorResult.value),
+              iteratorResult.done ? null : new Dirent(
+                iteratorResult.value.name,
+                this.path,
+                iteratorResult.value,
+              ),
             );
           }
         },
@@ -75,7 +83,11 @@ export default class Dir {
     if (iteratorResult.done) {
       return null;
     } else {
-      return new Dirent(iteratorResult.value);
+      return new Dirent(
+        iteratorResult.value.name,
+        this.path,
+        iteratorResult.value,
+      );
     }
   }
 

ext/node/polyfills/_fs/_fs_dirent.ts

@@ -2,7 +2,14 @@
 import { notImplemented } from "ext:deno_node/_utils.ts";
 
 export default class Dirent {
-  constructor(private entry: Deno.DirEntry & { parentPath: string }) {}
+  constructor(
+    // This is the most frequently accessed property. Using a non-getter
+    // is a very tiny bit faster here
+    public name: string,
+    public parentPath: string,
+    private entry: Deno.DirEntry,
+  ) {
+  }
 
   isBlockDevice(): boolean {
     notImplemented("Deno does not yet support identification of block devices");
@@ -37,15 +44,7 @@ export default class Dirent {
   }
 
   isSymbolicLink(): boolean {
-    return this.entry.isSymlink;
-  }
-
-  get name(): string | null {
-    return this.entry.name;
-  }
-
-  get parentPath(): string {
-    return this.entry.parentPath;
+    return this.entry.isSymLink;
   }
 
   /** @deprecated */

ext/node/polyfills/_fs/_fs_readdir.ts

@@ -3,17 +3,18 @@
 // TODO(petamoriken): enable prefer-primordials for node polyfills
 // deno-lint-ignore-file prefer-primordials
 
-import { TextDecoder, TextEncoder } from "ext:deno_web/08_text_encoding.js";
-import { asyncIterableToCallback } from "ext:deno_node/_fs/_fs_watch.ts";
+import { TextDecoder } from "ext:deno_web/08_text_encoding.js";
 import Dirent from "ext:deno_node/_fs/_fs_dirent.ts";
 import { denoErrorToNodeError } from "ext:deno_node/internal/errors.ts";
 import { getValidatedPath } from "ext:deno_node/internal/fs/utils.mjs";
 import { Buffer } from "node:buffer";
 import { promisify } from "ext:deno_node/internal/util.mjs";
-
-function toDirent(val: Deno.DirEntry & { parentPath: string }): Dirent {
-  return new Dirent(val);
-}
+import {
+  op_fs_read_dir_async,
+  op_fs_read_dir_names_async,
+  op_fs_read_dir_names_sync,
+  op_fs_read_dir_sync,
+} from "ext:core/ops";
 
 type readDirOptions = {
   encoding?: string;
@@ -51,7 +52,6 @@ export function readdir(
   const options = typeof optionsOrCallback === "object"
     ? optionsOrCallback
     : null;
-  const result: Array<string | Dirent> = [];
   path = getValidatedPath(path);
 
   if (!callback) throw new Error("No callback function supplied");
@@ -66,32 +66,31 @@ export function readdir(
     }
   }
 
-  try {
-    path = path.toString();
-    asyncIterableToCallback(Deno.readDir(path), (val, done) => {
-      if (typeof path !== "string") return;
-      if (done) {
-        callback(null, result);
-        return;
-      }
-      if (options?.withFileTypes) {
-        val.parentPath = path;
-        result.push(toDirent(val));
-      } else result.push(decode(val.name));
-    }, (e) => {
-      callback(denoErrorToNodeError(e as Error, { syscall: "readdir" }));
-    });
-  } catch (e) {
-    callback(denoErrorToNodeError(e as Error, { syscall: "readdir" }));
-  }
-}
+  path = path.toString();
+  if (options?.withFileTypes) {
+    op_fs_read_dir_async(path)
+      .then(
+        (files) => {
+          const result: Dirent[] = [];
 
-function decode(str: string, encoding?: string): string {
-  if (!encoding) return str;
-  else {
-    const decoder = new TextDecoder(encoding);
-    const encoder = new TextEncoder();
-    return decoder.decode(encoder.encode(str));
+          try {
+            for (let i = 0; i < files.length; i++) {
+              const file = files[i];
+              result.push(new Dirent(file.name, path, file));
+            }
+            callback(null, result);
+          } catch (e) {
+            callback(denoErrorToNodeError(e as Error, { syscall: "readdir" }));
+          }
+        },
+        (e: Error) => callback(denoErrorToNodeError(e, { syscall: "readdir" })),
+      );
+  } else {
+    op_fs_read_dir_names_async(path)
+      .then(
+        (fileNames) => callback(null, fileNames),
+        (e: Error) => callback(denoErrorToNodeError(e, { syscall: "readdir" })),
+      );
   }
 }
 
@@ -118,7 +117,6 @@ export function readdirSync(
   path: string | Buffer | URL,
   options?: readDirOptions,
 ): Array<string | Dirent> {
-  const result = [];
   path = getValidatedPath(path);
 
   if (options?.encoding) {
@@ -133,14 +131,18 @@ export function readdirSync(
 
   try {
     path = path.toString();
-    for (const file of Deno.readDirSync(path)) {
-      if (options?.withFileTypes) {
-        file.parentPath = path;
-        result.push(toDirent(file));
-      } else result.push(decode(file.name));
+    if (options?.withFileTypes) {
+      const result = [];
+      const files = op_fs_read_dir_sync(path);
+      for (let i = 0; i < files.length; i++) {
+        const file = files[i];
+        result.push(new Dirent(file.name, path, file));
+      }
+      return result;
+    } else {
+      return op_fs_read_dir_names_sync(path);
     }
   } catch (e) {
     throw denoErrorToNodeError(e as Error, { syscall: "readdir" });
   }
-  return result;
 }

ext/web/internal.d.ts

@@ -43,6 +43,7 @@ declare module "ext:deno_web/00_infra.js" {
   function forgivingBase64Decode(data: string): Uint8Array;
   function forgivingBase64UrlEncode(data: Uint8Array | string): string;
   function forgivingBase64UrlDecode(data: string): Uint8Array;
+  function pathFromURL(urlOrPath: string | URL): string;
   function serializeJSValueToJSONString(value: unknown): string;
 }
 

tests/unit_node/_fs/_fs_dirent_test.ts

@@ -1,12 +1,8 @@
 // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license.
 import { assert, assertEquals, assertThrows } from "@std/assert/mod.ts";
-import { Dirent as Dirent_ } from "node:fs";
-
-// deno-lint-ignore no-explicit-any
-const Dirent = Dirent_ as any;
+import { Dirent } from "node:fs";
 
 class DirEntryMock implements Deno.DirEntry {
-  parentPath = "";
   name = "";
   isFile = false;
   isDirectory = false;
@@ -16,66 +12,66 @@ class DirEntryMock implements Deno.DirEntry {
 Deno.test({
   name: "Directories are correctly identified",
   fn() {
-    const entry: DirEntryMock = new DirEntryMock();
+    const entry = new DirEntryMock();
     entry.isDirectory = true;
     entry.isFile = false;
     entry.isSymlink = false;
-    assert(new Dirent(entry).isDirectory());
-    assert(!new Dirent(entry).isFile());
-    assert(!new Dirent(entry).isSymbolicLink());
+    const dir = new Dirent("foo", "parent", entry);
+    assert(dir.isDirectory());
+    assert(!dir.isFile());
+    assert(!dir.isSymbolicLink());
   },
 });
 
 Deno.test({
   name: "Files are correctly identified",
   fn() {
-    const entry: DirEntryMock = new DirEntryMock();
+    const entry = new DirEntryMock();
     entry.isDirectory = false;
     entry.isFile = true;
     entry.isSymlink = false;
-    assert(!new Dirent(entry).isDirectory());
-    assert(new Dirent(entry).isFile());
-    assert(!new Dirent(entry).isSymbolicLink());
+    const dir = new Dirent("foo", "parent", entry);
+    assert(!dir.isDirectory());
+    assert(dir.isFile());
+    assert(!dir.isSymbolicLink());
   },
 });
 
 Deno.test({
   name: "Symlinks are correctly identified",
   fn() {
-    const entry: DirEntryMock = new DirEntryMock();
+    const entry = new DirEntryMock();
     entry.isDirectory = false;
     entry.isFile = false;
     entry.isSymlink = true;
-    assert(!new Dirent(entry).isDirectory());
-    assert(!new Dirent(entry).isFile());
-    assert(new Dirent(entry).isSymbolicLink());
+    const dir = new Dirent("foo", "parent", entry);
+    assert(!dir.isDirectory());
+    assert(!dir.isFile());
+    assert(dir.isSymbolicLink());
   },
 });
 
 Deno.test({
   name: "File name is correct",
   fn() {
-    const entry: DirEntryMock = new DirEntryMock();
-    entry.name = "my_file";
-    assertEquals(new Dirent(entry).name, "my_file");
+    const entry = new DirEntryMock();
+    const mock = new Dirent("my_file", "parent", entry);
+    assertEquals(mock.name, "my_file");
   },
 });
 
 Deno.test({
   name: "Socket and FIFO pipes aren't yet available",
   fn() {
-    const entry: DirEntryMock = new DirEntryMock();
+    const entry = new DirEntryMock();
+    const dir = new Dirent("my_file", "parent", entry);
     assertThrows(
-      () => {
-        new Dirent(entry).isFIFO();
-      },
+      () => dir.isFIFO(),
       Error,
       "does not yet support",
     );
     assertThrows(
-      () => {
-        new Dirent(entry).isSocket();
-      },
+      () => dir.isSocket(),
       Error,
       "does not yet support",
     );
@@ -85,11 +81,10 @@ Deno.test({
 Deno.test({
   name: "Path and parent path is correct",
   fn() {
-    const entry: DirEntryMock = new DirEntryMock();
-    entry.name = "my_file";
-    entry.parentPath = "/home/user";
-    assertEquals(new Dirent(entry).name, "my_file");
-    assertEquals(new Dirent(entry).path, "/home/user");
-    assertEquals(new Dirent(entry).parentPath, "/home/user");
+    const entry = new DirEntryMock();
+    const dir = new Dirent("my_file", "/home/user", entry);
+    assertEquals(dir.name, "my_file");
+    assertEquals(dir.path, "/home/user");
+    assertEquals(dir.parentPath, "/home/user");
   },
 });