fs: improve error performance of mkdtempSync

anonrig · targos · commit 275987841ed0 · 2023-10-23T08:19:36.000+02: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
@@ -2966,11 +2966,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
@@ -2760,27 +2760,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
`@@ -2966,11 +2966,7 @@ function mkdtempSync(prefix, options) {`
`2966`	`2966`	`path = Buffer.concat([prefix, Buffer.from('XXXXXX')]);`
`2967`	`2967`	`}`
`2968`	`2968`
`2969`		`- const ctx = { path };`
`2970`		`- const result = binding.mkdtemp(path, options.encoding,`
`2971`		`- undefined, ctx);`
`2972`		`- handleErrorFromBinding(ctx);`
`2973`		`- return result;`
	`2969`	`+ return binding.mkdtemp(path, options.encoding);`
`2974`	`2970`	`}`
`2975`	`2971`
`2976`	`2972`	`/**`