Skip to content

Commit b826f39

Browse files
CanadaHonkanonrig
authored andcommitted
fs: improve error performance for mkdirSync
1 parent dc1c50b commit b826f39

File tree

4 files changed

+60
-18
lines changed

4 files changed

+60
-18
lines changed

benchmark/fs/bench-mkdirSync.js

+44
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
const fs = require('fs');
5+
const tmpdir = require('../../test/common/tmpdir');
6+
tmpdir.refresh();
7+
8+
const bench = common.createBenchmark(main, {
9+
type: ['existing', 'non-existing'],
10+
recursive: ['true', 'false'],
11+
n: [1e3],
12+
});
13+
14+
function main({ n, type, recursive }) {
15+
recursive = recursive === 'true';
16+
let files;
17+
18+
switch (type) {
19+
case 'non-existing':
20+
files = [];
21+
22+
// Populate tmpdir with target dirs
23+
for (let i = 0; i < n; i++) {
24+
const path = tmpdir.resolve(recursive ? `rmdirsync-bench-dir-${process.pid}-${i}/a/b/c` : `rmdirsync-bench-dir-${process.pid}-${i}`);
25+
files.push(path);
26+
}
27+
break;
28+
case 'existing':
29+
files = new Array(n).fill(__dirname);
30+
break;
31+
default:
32+
new Error('Invalid type');
33+
}
34+
35+
bench.start();
36+
for (let i = 0; i < n; i++) {
37+
try {
38+
fs.mkdirSync(files[i], { recursive });
39+
} catch {
40+
// do nothing
41+
}
42+
}
43+
bench.end(n);
44+
}

lib/fs.js

+6-5
Original file line numberDiff line numberDiff line change
@@ -1390,11 +1390,12 @@ function mkdirSync(path, options) {
13901390
path = getValidatedPath(path);
13911391
validateBoolean(recursive, 'options.recursive');
13921392

1393-
const ctx = { path };
1394-
const result = binding.mkdir(pathModule.toNamespacedPath(path),
1395-
parseFileMode(mode, 'mode'), recursive,
1396-
undefined, ctx);
1397-
handleErrorFromBinding(ctx);
1393+
const result = binding.mkdir(
1394+
pathModule.toNamespacedPath(path),
1395+
parseFileMode(mode, 'mode'),
1396+
recursive,
1397+
);
1398+
13981399
if (recursive) {
13991400
return result;
14001401
}

src/node_file.cc

+7-10
Original file line numberDiff line numberDiff line change
@@ -1780,7 +1780,7 @@ static void MKDir(const FunctionCallbackInfo<Value>& args) {
17801780
Environment* env = Environment::GetCurrent(args);
17811781

17821782
const int argc = args.Length();
1783-
CHECK_GE(argc, 4);
1783+
CHECK_GE(argc, 3);
17841784

17851785
BufferValue path(env->isolate(), args[0]);
17861786
CHECK_NOT_NULL(*path);
@@ -1793,16 +1793,15 @@ static void MKDir(const FunctionCallbackInfo<Value>& args) {
17931793
CHECK(args[2]->IsBoolean());
17941794
bool mkdirp = args[2]->IsTrue();
17951795

1796-
FSReqBase* req_wrap_async = GetReqWrap(args, 3);
1797-
if (req_wrap_async != nullptr) { // mkdir(path, mode, req)
1796+
if (argc > 3) { // mkdir(path, mode, recursive, req)
1797+
FSReqBase* req_wrap_async = GetReqWrap(args, 3);
17981798
FS_ASYNC_TRACE_BEGIN1(
17991799
UV_FS_UNLINK, req_wrap_async, "path", TRACE_STR_COPY(*path))
18001800
AsyncCall(env, req_wrap_async, args, "mkdir", UTF8,
18011801
mkdirp ? AfterMkdirp : AfterNoArgs,
18021802
mkdirp ? MKDirpAsync : uv_fs_mkdir, *path, mode);
1803-
} else { // mkdir(path, mode, undefined, ctx)
1804-
CHECK_EQ(argc, 5);
1805-
FSReqWrapSync req_wrap_sync;
1803+
} else { // mkdir(path, mode, recursive)
1804+
FSReqWrapSync req_wrap_sync("mkdir", *path);
18061805
FS_SYNC_TRACE_BEGIN(mkdir);
18071806
if (mkdirp) {
18081807
int err = CallMKDirpSync(env, args, &req_wrap_sync, *path, mode);
@@ -1815,15 +1814,13 @@ static void MKDir(const FunctionCallbackInfo<Value>& args) {
18151814
first_path.c_str(),
18161815
UTF8, &error);
18171816
if (path.IsEmpty()) {
1818-
Local<Object> ctx = args[4].As<Object>();
1819-
ctx->Set(env->context(), env->error_string(), error).Check();
1817+
env->isolate()->ThrowException(error);
18201818
return;
18211819
}
18221820
args.GetReturnValue().Set(path.ToLocalChecked());
18231821
}
18241822
} else {
1825-
SyncCall(env, args[4], &req_wrap_sync, "mkdir",
1826-
uv_fs_mkdir, *path, mode);
1823+
SyncCallAndThrowOnError(env, &req_wrap_sync, uv_fs_mkdir, *path, mode);
18271824
}
18281825
FS_SYNC_TRACE_END(mkdir);
18291826
}

typings/internalBinding/fs.d.ts

+3-3
Original file line numberDiff line numberDiff line change
@@ -142,9 +142,9 @@ declare namespace InternalFSBinding {
142142
function mkdir(path: string, mode: number, recursive: boolean, req: FSReqCallback<void | string>): void;
143143
function mkdir(path: string, mode: number, recursive: true, req: FSReqCallback<string>): void;
144144
function mkdir(path: string, mode: number, recursive: false, req: FSReqCallback<void>): void;
145-
function mkdir(path: string, mode: number, recursive: boolean, req: undefined, ctx: FSSyncContext): void | string;
146-
function mkdir(path: string, mode: number, recursive: true, req: undefined, ctx: FSSyncContext): string;
147-
function mkdir(path: string, mode: number, recursive: false, req: undefined, ctx: FSSyncContext): void;
145+
function mkdir(path: string, mode: number, recursive: boolean): void | string;
146+
function mkdir(path: string, mode: number, recursive: true): string;
147+
function mkdir(path: string, mode: number, recursive: false): void;
148148
function mkdir(path: string, mode: number, recursive: boolean, usePromises: typeof kUsePromises): Promise<void | string>;
149149
function mkdir(path: string, mode: number, recursive: true, usePromises: typeof kUsePromises): Promise<string>;
150150
function mkdir(path: string, mode: number, recursive: false, usePromises: typeof kUsePromises): Promise<void>;

0 commit comments

Comments
 (0)