Skip to content

Commit

Permalink
fs: improve error performance for mkdirSync
Browse files Browse the repository at this point in the history
PR-URL: nodejs#49847
Refs: nodejs/performance#106
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
  • Loading branch information
CanadaHonk authored and alexfernandez committed Nov 1, 2023
1 parent d1141f4 commit a4756a6
Show file tree
Hide file tree
Showing 4 changed files with 68 additions and 40 deletions.
44 changes: 44 additions & 0 deletions benchmark/fs/bench-mkdirSync.js
Original file line number Diff line number Diff line change
@@ -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);
}
11 changes: 6 additions & 5 deletions lib/fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
47 changes: 15 additions & 32 deletions src/node_file.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
}
Expand Down
6 changes: 3 additions & 3 deletions typings/internalBinding/fs.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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>;
Expand Down

0 comments on commit a4756a6

Please sign in to comment.