fs: improve error performance of mkdtempSync

anonrig · targos · commit 12cda31c5228 · 2023-11-11T20:29:13.000+01:00
PR-URL: #49962 Refs: nodejs/performance#106 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Stephen Belanger <admin@stephenbelanger.com> Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
diff --git a/lib/fs.js b/lib/fs.js
@@ -2964,11 +2964,7 @@ function mkdtempSync(prefix, options) {
     path = Buffer.concat([prefix, Buffer.from('XXXXXX')]);
   }
 
-  const ctx = { path };
-  const result = binding.mkdtemp(path, options.encoding,
-                                 undefined, ctx);
-  handleErrorFromBinding(ctx);
-  return result;
+  return binding.mkdtemp(path, options.encoding);
 }
 
 /**
diff --git a/src/node_file.cc b/src/node_file.cc
@@ -2795,27 +2795,26 @@ static void Mkdtemp(const FunctionCallbackInfo<Value>& args) {
 
   const enum encoding encoding = ParseEncoding(isolate, args[1], UTF8);
 
-  FSReqBase* req_wrap_async = GetReqWrap(args, 2);
-  if (req_wrap_async != nullptr) {  // mkdtemp(tmpl, encoding, req)
+  if (argc > 2) {  // mkdtemp(tmpl, encoding, req)
+    FSReqBase* req_wrap_async = GetReqWrap(args, 2);
     FS_ASYNC_TRACE_BEGIN1(
         UV_FS_MKDTEMP, req_wrap_async, "path", TRACE_STR_COPY(*tmpl))
     AsyncCall(env, req_wrap_async, args, "mkdtemp", encoding, AfterStringPath,
               uv_fs_mkdtemp, *tmpl);
-  } else {  // mkdtemp(tmpl, encoding, undefined, ctx)
-    CHECK_EQ(argc, 4);
-    FSReqWrapSync req_wrap_sync;
+  } else {  // mkdtemp(tmpl, encoding)
+    FSReqWrapSync req_wrap_sync("mkdtemp", *tmpl);
     FS_SYNC_TRACE_BEGIN(mkdtemp);
-    SyncCall(env, args[3], &req_wrap_sync, "mkdtemp",
-             uv_fs_mkdtemp, *tmpl);
+    int result =
+        SyncCallAndThrowOnError(env, &req_wrap_sync, uv_fs_mkdtemp, *tmpl);
     FS_SYNC_TRACE_END(mkdtemp);
-    const char* path = req_wrap_sync.req.path;
-
+    if (is_uv_error(result)) {
+      return;
+    }
     Local<Value> error;
     MaybeLocal<Value> rc =
-        StringBytes::Encode(isolate, path, encoding, &error);
+        StringBytes::Encode(isolate, req_wrap_sync.req.path, encoding, &error);
     if (rc.IsEmpty()) {
-      Local<Object> ctx = args[3].As<Object>();
-      ctx->Set(env->context(), env->error_string(), error).Check();
+      env->isolate()->ThrowException(error);
       return;
     }
     args.GetReturnValue().Set(rc.ToLocalChecked());
diff --git a/typings/internalBinding/fs.d.ts b/typings/internalBinding/fs.d.ts
@@ -140,6 +140,7 @@ declare namespace InternalFSBinding {
   function mkdtemp(prefix: string, encoding: unknown, req: FSReqCallback<string>): void;
   function mkdtemp(prefix: string, encoding: unknown, req: undefined, ctx: FSSyncContext): string;
   function mkdtemp(prefix: string, encoding: unknown, usePromises: typeof kUsePromises): Promise<string>;
+  function mkdtemp(prefix: string, encoding: unknown): string;
 
   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;

Original file line number	Diff line number	Diff line change
`@@ -2964,11 +2964,7 @@ function mkdtempSync(prefix, options) {`
`2964`	`2964`	`path = Buffer.concat([prefix, Buffer.from('XXXXXX')]);`
`2965`	`2965`	`}`
`2966`	`2966`
`2967`		`- const ctx = { path };`
`2968`		`- const result = binding.mkdtemp(path, options.encoding,`
`2969`		`- undefined, ctx);`
`2970`		`- handleErrorFromBinding(ctx);`
`2971`		`- return result;`
	`2967`	`+ return binding.mkdtemp(path, options.encoding);`
`2972`	`2968`	`}`
`2973`	`2969`
`2974`	`2970`	`/**`