Skip to content

Commit 78557c1

Browse files
CanadaHonkanonrig
authored andcommitted
fs: improve error performance for rmdirSync
1 parent dc1c50b commit 78557c1

File tree

4 files changed

+50
-12
lines changed

4 files changed

+50
-12
lines changed

benchmark/fs/bench-rmdirSync.js

+42
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
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+
n: [1e3],
11+
});
12+
13+
function main({ n, type }) {
14+
switch (type) {
15+
case 'existing': {
16+
for (let i = 0; i < n; i++) {
17+
fs.mkdirSync(tmpdir.resolve(`rmdirsync-bench-dir-${i}`));
18+
}
19+
20+
bench.start();
21+
for (let i = 0; i < n; i++) {
22+
fs.rmdirSync(tmpdir.resolve(`rmdirsync-bench-dir-${i}`));
23+
}
24+
bench.end(n);
25+
break;
26+
}
27+
case 'non-existing': {
28+
bench.start();
29+
for (let i = 0; i < n; i++) {
30+
try {
31+
fs.rmdirSync(tmpdir.resolve(`.non-existent-folder-${i}`));
32+
} catch {
33+
// do nothing
34+
}
35+
}
36+
bench.end(n);
37+
break;
38+
}
39+
default:
40+
new Error('Invalid type');
41+
}
42+
}

lib/fs.js

+1-3
Original file line numberDiff line numberDiff line change
@@ -1225,9 +1225,7 @@ function rmdirSync(path, options) {
12251225
validateRmdirOptions(options);
12261226
}
12271227

1228-
const ctx = { path };
1229-
binding.rmdir(pathModule.toNamespacedPath(path), undefined, ctx);
1230-
return handleErrorFromBinding(ctx);
1228+
binding.rmdir(pathModule.toNamespacedPath(path));
12311229
}
12321230

12331231
/**

src/node_file.cc

+6-8
Original file line numberDiff line numberDiff line change
@@ -1571,25 +1571,23 @@ static void RMDir(const FunctionCallbackInfo<Value>& args) {
15711571
Environment* env = Environment::GetCurrent(args);
15721572

15731573
const int argc = args.Length();
1574-
CHECK_GE(argc, 2);
1574+
CHECK_GE(argc, 1);
15751575

15761576
BufferValue path(env->isolate(), args[0]);
15771577
CHECK_NOT_NULL(*path);
15781578
THROW_IF_INSUFFICIENT_PERMISSIONS(
15791579
env, permission::PermissionScope::kFileSystemWrite, path.ToStringView());
15801580

1581-
FSReqBase* req_wrap_async = GetReqWrap(args, 1); // rmdir(path, req)
1582-
if (req_wrap_async != nullptr) {
1581+
if (argc > 1) {
1582+
FSReqBase* req_wrap_async = GetReqWrap(args, 1); // rmdir(path, req)
15831583
FS_ASYNC_TRACE_BEGIN1(
15841584
UV_FS_RMDIR, req_wrap_async, "path", TRACE_STR_COPY(*path))
15851585
AsyncCall(env, req_wrap_async, args, "rmdir", UTF8, AfterNoArgs,
15861586
uv_fs_rmdir, *path);
1587-
} else { // rmdir(path, undefined, ctx)
1588-
CHECK_EQ(argc, 3);
1589-
FSReqWrapSync req_wrap_sync;
1587+
} else { // rmdir(path)
1588+
FSReqWrapSync req_wrap_sync("rmdir", *path);
15901589
FS_SYNC_TRACE_BEGIN(rmdir);
1591-
SyncCall(env, args[2], &req_wrap_sync, "rmdir",
1592-
uv_fs_rmdir, *path);
1590+
SyncCallAndThrowOnError(env, &req_wrap_sync, uv_fs_rmdir, *path);
15931591
FS_SYNC_TRACE_END(rmdir);
15941592
}
15951593
}

typings/internalBinding/fs.d.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,7 @@ declare namespace InternalFSBinding {
185185
function rename(oldPath: string, newPath: string, usePromises: typeof kUsePromises): Promise<void>;
186186

187187
function rmdir(path: string, req: FSReqCallback): void;
188-
function rmdir(path: string, req: undefined, ctx: FSSyncContext): void;
188+
function rmdir(path: string): void;
189189
function rmdir(path: string, usePromises: typeof kUsePromises): Promise<void>;
190190

191191
function stat(path: StringOrBuffer, useBigint: boolean, req: FSReqCallback<Float64Array | BigUint64Array>): void;

0 commit comments

Comments
 (0)