fs: improve error performance for mkdirSync

nodejs · Oct 11, 2023 · 92b3a70 · 92b3a70
1 parent dc1c50b
commit 92b3a70
Show file tree

Hide file tree

Showing 4 changed files with 60 additions and 18 deletions.
diff --git a/benchmark/fs/bench-mkdirSync.js b/benchmark/fs/bench-mkdirSync.js
@@ -0,0 +1,44 @@
+'use strict';
+
+const common = require('../common');
+const fs = require('fs');
+const tmpdir = require('../../test/common/tmpdir');
+tmpdir.refresh();
+
+const bench = common.createBenchmark(main, {
+  type: ['existing', 'non-existing'],
+  recursive: ['true', 'false'],
+  n: [1e3],
+});
+
+function main({ n, type, recursive }) {
+  recursive = recursive === 'true';
+  let files;
+
+  switch (type) {
+    case 'non-existing':
+      files = [];
+
+      // Populate tmpdir with target dirs
+      for (let i = 0; i < n; i++) {
+        const path = tmpdir.resolve(recursive ? `rmdirsync-bench-dir-${process.pid}-${i}/a/b/c` : `rmdirsync-bench-dir-${process.pid}-${i}`);
+        files.push(path);
+      }
+      break;
+    case 'existing':
+      files = new Array(n).fill(__dirname);
+      break;
+    default:
+      new Error('Invalid type');
+  }
+
+  bench.start();
+  for (let i = 0; i < n; i++) {
+    try {
+      fs.mkdirSync(files[i], { recursive });
+    } catch {
+      // do nothing
+    }
+  }
+  bench.end(n);
+}
diff --git a/lib/fs.js b/lib/fs.js
@@ -1390,11 +1390,12 @@ function mkdirSync(path, options) {
   path = getValidatedPath(path);
   validateBoolean(recursive, 'options.recursive');
 
-  const ctx = { path };
-  const result = binding.mkdir(pathModule.toNamespacedPath(path),
-                               parseFileMode(mode, 'mode'), recursive,
-                               undefined, ctx);
-  handleErrorFromBinding(ctx);
+  const result = binding.mkdir(
+    pathModule.toNamespacedPath(path),
+    parseFileMode(mode, 'mode'),
+    recursive,
+  );
+
   if (recursive) {
     return result;
   }

diff --git a/src/node_file.cc b/src/node_file.cc
@@ -1780,7 +1780,7 @@ static void MKDir(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
 
   const int argc = args.Length();
-  CHECK_GE(argc, 4);
+  CHECK_GE(argc, 2);
 
   BufferValue path(env->isolate(), args[0]);
   CHECK_NOT_NULL(*path);
@@ -1793,16 +1793,15 @@ static void MKDir(const FunctionCallbackInfo<Value>& args) {
   CHECK(args[2]->IsBoolean());
   bool mkdirp = args[2]->IsTrue();
 
-  FSReqBase* req_wrap_async = GetReqWrap(args, 3);
-  if (req_wrap_async != nullptr) {  // mkdir(path, mode, req)
+  if (argc > 2) {  // mkdir(path, mode, req)
+    FSReqBase* req_wrap_async = GetReqWrap(args, 3);
     FS_ASYNC_TRACE_BEGIN1(
         UV_FS_UNLINK, req_wrap_async, "path", TRACE_STR_COPY(*path))
     AsyncCall(env, req_wrap_async, args, "mkdir", UTF8,
               mkdirp ? AfterMkdirp : AfterNoArgs,
               mkdirp ? MKDirpAsync : uv_fs_mkdir, *path, mode);
-  } else {  // mkdir(path, mode, undefined, ctx)
-    CHECK_EQ(argc, 5);
-    FSReqWrapSync req_wrap_sync;
+  } else {  // mkdir(path, mode)
+    FSReqWrapSync req_wrap_sync("mkdir", *path);
     FS_SYNC_TRACE_BEGIN(mkdir);
     if (mkdirp) {
       int err = CallMKDirpSync(env, args, &req_wrap_sync, *path, mode);
@@ -1815,15 +1814,13 @@ static void MKDir(const FunctionCallbackInfo<Value>& args) {
                                                      first_path.c_str(),
                                                      UTF8, &error);
         if (path.IsEmpty()) {
-          Local<Object> ctx = args[4].As<Object>();
-          ctx->Set(env->context(), env->error_string(), error).Check();
+          env->isolate()->ThrowException(error);
           return;
         }
         args.GetReturnValue().Set(path.ToLocalChecked());
       }
     } else {
-      SyncCall(env, args[4], &req_wrap_sync, "mkdir",
-               uv_fs_mkdir, *path, mode);
+      SyncCallAndThrowOnError(env, &req_wrap_sync, uv_fs_mkdir, *path, mode);
     }
     FS_SYNC_TRACE_END(mkdir);
   }

diff --git a/typings/internalBinding/fs.d.ts b/typings/internalBinding/fs.d.ts
@@ -142,9 +142,9 @@ declare namespace InternalFSBinding {
   function mkdir(path: string, mode: number, recursive: boolean, req: FSReqCallback<void | string>): void;
   function mkdir(path: string, mode: number, recursive: true, req: FSReqCallback<string>): void;
   function mkdir(path: string, mode: number, recursive: false, req: FSReqCallback<void>): void;
-  function mkdir(path: string, mode: number, recursive: boolean, req: undefined, ctx: FSSyncContext): void | string;
-  function mkdir(path: string, mode: number, recursive: true, req: undefined, ctx: FSSyncContext): string;
-  function mkdir(path: string, mode: number, recursive: false, req: undefined, ctx: FSSyncContext): void;
+  function mkdir(path: string, mode: number, recursive: boolean): void | string;
+  function mkdir(path: string, mode: number, recursive: true): string;
+  function mkdir(path: string, mode: number, recursive: false): void;
   function mkdir(path: string, mode: number, recursive: boolean, usePromises: typeof kUsePromises): Promise<void | string>;
   function mkdir(path: string, mode: number, recursive: true, usePromises: typeof kUsePromises): Promise<string>;
   function mkdir(path: string, mode: number, recursive: false, usePromises: typeof kUsePromises): Promise<void>;