fs: improve error performance for mkdirSync

PR-URL: #49847
Refs: nodejs/performance#106
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>

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);
+}

lib/fs.js

@@ -1385,11 +1385,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;
  }

src/node_file.cc

@@ -1751,30 +1751,11 @@ int MKDirpAsync(uv_loop_t* loop,
  return err;
 }
 
-int CallMKDirpSync(Environment* env, const FunctionCallbackInfo<Value>& args,
- FSReqWrapSync* req_wrap, const char* path, int mode) {
- env->PrintSyncTrace();
- int err = MKDirpSync(env->event_loop(), &req_wrap->req, path, mode,
- nullptr);
- if (err < 0) {
- v8::Local<v8::Context> context = env->context();
- v8::Local<v8::Object> ctx_obj = args[4].As<v8::Object>();
- v8::Isolate* isolate = env->isolate();
- ctx_obj->Set(context,
- env->errno_string(),
- v8::Integer::New(isolate, err)).Check();
- ctx_obj->Set(context,
- env->syscall_string(),
- OneByteString(isolate, "mkdir")).Check();
- }
- return err;
-}
-
 static void MKDir(const FunctionCallbackInfo<Value>& args) {
  Environment* env = Environment::GetCurrent(args);
 
  const int argc = args.Length();
- CHECK_GE(argc, 4);
+ CHECK_GE(argc, 3);
 
  BufferValue path(env->isolate(), args[0]);
  CHECK_NOT_NULL(*path);
@@ -1787,37 +1768,39 @@ 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 > 3) { // mkdir(path, mode, recursive, 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, recursive)
+ FSReqWrapSync req_wrap_sync("mkdir", *path);
  FS_SYNC_TRACE_BEGIN(mkdir);
  if (mkdirp) {
- int err = CallMKDirpSync(env, args, &req_wrap_sync, *path, mode);
- if (err == 0 &&
- !req_wrap_sync.continuation_data()->first_path().empty()) {
+ env->PrintSyncTrace();
+ int err = MKDirpSync(
+ env->event_loop(), &req_wrap_sync.req, *path, mode, nullptr);
+ if (is_uv_error(err)) {
+ env->ThrowUVException(err, "mkdir", nullptr, *path);
+ return;
+ }
+ if (!req_wrap_sync.continuation_data()->first_path().empty()) {
  Local<Value> error;
  std::string first_path(req_wrap_sync.continuation_data()->first_path());
  FromNamespacedPath(&first_path);
  MaybeLocal<Value> path = StringBytes::Encode(env->isolate(),
  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);
  }

typings/internalBinding/fs.d.ts

@@ -146,9 +146,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>;