From 12cda31c5228306fd3140579a32ff317fa0f50db Mon Sep 17 00:00:00 2001 From: Yagiz Nizipli Date: Fri, 29 Sep 2023 13:11:44 -0400 Subject: [PATCH] fs: improve error performance of `mkdtempSync` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-URL: https://github.com/nodejs/node/pull/49962 Refs: https://github.com/nodejs/performance/issues/106 Reviewed-By: Matteo Collina Reviewed-By: Stephen Belanger Reviewed-By: Vinícius Lourenço Claro Cardoso --- lib/fs.js | 6 +----- src/node_file.cc | 23 +++++++++++------------ typings/internalBinding/fs.d.ts | 1 + 3 files changed, 13 insertions(+), 17 deletions(-) diff --git a/lib/fs.js b/lib/fs.js index 1943f6893901ad..16920576841fab 100644 --- 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 index 246c938ccfb240..d601c713686bb8 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -2795,27 +2795,26 @@ static void Mkdtemp(const FunctionCallbackInfo& 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 error; MaybeLocal rc = - StringBytes::Encode(isolate, path, encoding, &error); + StringBytes::Encode(isolate, req_wrap_sync.req.path, encoding, &error); if (rc.IsEmpty()) { - Local ctx = args[3].As(); - 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 index 23592809f2c570..2c03c9934d2775 100644 --- 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): void; function mkdtemp(prefix: string, encoding: unknown, req: undefined, ctx: FSSyncContext): string; function mkdtemp(prefix: string, encoding: unknown, usePromises: typeof kUsePromises): Promise; + function mkdtemp(prefix: string, encoding: unknown): string; function mkdir(path: string, mode: number, recursive: boolean, req: FSReqCallback): void; function mkdir(path: string, mode: number, recursive: true, req: FSReqCallback): void;