From b618fe262f973bef75dfb8d7bdcd5893575be389 Mon Sep 17 00:00:00 2001
From: Yagiz Nizipli <yagiz@nizipli.com>
Date: Thu, 21 Sep 2023 13:04:13 -0400
Subject: [PATCH] fs: improve error performance of `opendirSync`

PR-URL: https://github.com/nodejs/node/pull/49705
Refs: https://github.com/nodejs/performance/issues/106
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
---
 benchmark/fs/bench-opendirSync.js | 43 +++++++++++++++++++++++++++++++
 lib/internal/fs/dir.js            | 11 ++------
 src/node_dir.cc                   | 28 ++++++++++++++++++++
 3 files changed, 73 insertions(+), 9 deletions(-)
 create mode 100644 benchmark/fs/bench-opendirSync.js

diff --git a/benchmark/fs/bench-opendirSync.js b/benchmark/fs/bench-opendirSync.js
new file mode 100644
index 00000000000000..206822db139ff7
--- /dev/null
+++ b/benchmark/fs/bench-opendirSync.js
@@ -0,0 +1,43 @@
+'use strict';
+
+const common = require('../common');
+const fs = require('fs');
+const path = require('path');
+const tmpdir = require('../../test/common/tmpdir');
+tmpdir.refresh();
+
+const testFiles = fs.readdirSync('test', { withFileTypes: true })
+  .filter((f) => f.isDirectory())
+  .map((f) => path.join(f.path, f.name));
+const bench = common.createBenchmark(main, {
+  type: ['existing', 'non-existing'],
+  n: [1e3],
+});
+
+function main({ n, type }) {
+  let files;
+
+  switch (type) {
+    case 'existing':
+      files = testFiles;
+      break;
+    case 'non-existing':
+      files = [tmpdir.resolve(`.non-existing-file-${Date.now()}`)];
+      break;
+    default:
+      new Error('Invalid type');
+  }
+
+  bench.start();
+  for (let i = 0; i < n; i++) {
+    for (let j = 0; j < files.length; j++) {
+      try {
+        const dir = fs.opendirSync(files[j]);
+        dir.closeSync();
+      } catch {
+        // do nothing
+      }
+    }
+  }
+  bench.end(n);
+}
diff --git a/lib/internal/fs/dir.js b/lib/internal/fs/dir.js
index a443532d35d3ed..1118ff5f674915 100644
--- a/lib/internal/fs/dir.js
+++ b/lib/internal/fs/dir.js
@@ -322,18 +322,11 @@ function opendir(path, options, callback) {
 
 function opendirSync(path, options) {
   path = getValidatedPath(path);
-  options = getOptions(options, {
-    encoding: 'utf8',
-  });
+  options = getOptions(options, { encoding: 'utf8' });
 
-  const ctx = { path };
-  const handle = dirBinding.opendir(
+  const handle = dirBinding.opendirSync(
     pathModule.toNamespacedPath(path),
-    options.encoding,
-    undefined,
-    ctx,
   );
-  handleErrorFromBinding(ctx);
 
   return new Dir(handle, path, options);
 }
diff --git a/src/node_dir.cc b/src/node_dir.cc
index 8f93f189cfbe27..10cde6067899c7 100644
--- a/src/node_dir.cc
+++ b/src/node_dir.cc
@@ -397,6 +397,32 @@ static void OpenDir(const FunctionCallbackInfo<Value>& args) {
   }
 }
 
+static void OpenDirSync(const FunctionCallbackInfo<Value>& args) {
+  Environment* env = Environment::GetCurrent(args);
+  Isolate* isolate = env->isolate();
+
+  CHECK_GE(args.Length(), 1);
+
+  BufferValue path(isolate, args[0]);
+  CHECK_NOT_NULL(*path);
+  THROW_IF_INSUFFICIENT_PERMISSIONS(
+      env, permission::PermissionScope::kFileSystemRead, path.ToStringView());
+
+  uv_fs_t req;
+  auto make = OnScopeLeave([&req]() { uv_fs_req_cleanup(&req); });
+  FS_DIR_SYNC_TRACE_BEGIN(opendir);
+  int err = uv_fs_opendir(nullptr, &req, *path, nullptr);
+  FS_DIR_SYNC_TRACE_END(opendir);
+  if (err < 0) {
+    return env->ThrowUVException(err, "opendir");
+  }
+
+  uv_dir_t* dir = static_cast<uv_dir_t*>(req.ptr);
+  DirHandle* handle = DirHandle::New(env, dir);
+
+  args.GetReturnValue().Set(handle->object().As<Value>());
+}
+
 void Initialize(Local<Object> target,
                 Local<Value> unused,
                 Local<Context> context,
@@ -405,6 +431,7 @@ void Initialize(Local<Object> target,
   Isolate* isolate = env->isolate();
 
   SetMethod(context, target, "opendir", OpenDir);
+  SetMethod(context, target, "opendirSync", OpenDirSync);
 
   // Create FunctionTemplate for DirHandle
   Local<FunctionTemplate> dir = NewFunctionTemplate(isolate, DirHandle::New);
@@ -419,6 +446,7 @@ void Initialize(Local<Object> target,
 
 void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
   registry->Register(OpenDir);
+  registry->Register(OpenDirSync);
   registry->Register(DirHandle::New);
   registry->Register(DirHandle::Read);
   registry->Register(DirHandle::Close);