Skip to content

Commit 483200f

Browse files
CanadaHonkRafaelGSS
authored andcommitted
fs: improve error performance for rmdirSync
PR-URL: #49846 Refs: nodejs/performance#106 Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Vinícius Lourenço Claro Cardoso <[email protected]>
1 parent 1a0069b commit 483200f

File tree

4 files changed

+50
-12
lines changed

4 files changed

+50
-12
lines changed

‎benchmark/fs/bench-rmdirSync.js‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
'use strict';
2+
3+
constcommon=require('../common');
4+
constfs=require('fs');
5+
consttmpdir=require('../../test/common/tmpdir');
6+
tmpdir.refresh();
7+
8+
constbench=common.createBenchmark(main,{
9+
type: ['existing','non-existing'],
10+
n: [1e4],
11+
});
12+
13+
functionmain({ n, type }){
14+
switch(type){
15+
case'existing': {
16+
for(leti=0;i<n;i++){
17+
fs.mkdirSync(tmpdir.resolve(`rmdirsync-bench-dir-${i}`));
18+
}
19+
20+
bench.start();
21+
for(leti=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(leti=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+
newError('Invalid type');
41+
}
42+
}

‎lib/fs.js‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1218,9 +1218,7 @@ function rmdirSync(path, options){
12181218
validateRmdirOptions(options);
12191219
}
12201220

1221-
constctx={ path };
1222-
binding.rmdir(pathModule.toNamespacedPath(path),undefined,ctx);
1223-
returnhandleErrorFromBinding(ctx);
1221+
binding.rmdir(pathModule.toNamespacedPath(path));
12241222
}
12251223

12261224
/**

‎src/node_file.cc‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1562,25 +1562,23 @@ static void RMDir(const FunctionCallbackInfo<Value>& args){
15621562
Environment* env = Environment::GetCurrent(args);
15631563

15641564
constint argc = args.Length();
1565-
CHECK_GE(argc, 2);
1565+
CHECK_GE(argc, 1);
15661566

15671567
BufferValue path(env->isolate(), args[0]);
15681568
CHECK_NOT_NULL(*path);
15691569
THROW_IF_INSUFFICIENT_PERMISSIONS(
15701570
env, permission::PermissionScope::kFileSystemWrite, path.ToStringView());
15711571

1572-
FSReqBase* req_wrap_async = GetReqWrap(args, 1); // rmdir(path, req)
1573-
if (req_wrap_async != nullptr){
1572+
if (argc > 1){
1573+
FSReqBase* req_wrap_async = GetReqWrap(args, 1); // rmdir(path, req)
15741574
FS_ASYNC_TRACE_BEGIN1(
15751575
UV_FS_RMDIR, req_wrap_async, "path", TRACE_STR_COPY(*path))
15761576
AsyncCall(env, req_wrap_async, args, "rmdir", UTF8, AfterNoArgs,
15771577
uv_fs_rmdir, *path);
1578-
} else{// rmdir(path, undefined, ctx)
1579-
CHECK_EQ(argc, 3);
1580-
FSReqWrapSync req_wrap_sync;
1578+
} else{// rmdir(path)
1579+
FSReqWrapSync req_wrap_sync("rmdir", *path);
15811580
FS_SYNC_TRACE_BEGIN(rmdir);
1582-
SyncCall(env, args[2], &req_wrap_sync, "rmdir",
1583-
uv_fs_rmdir, *path);
1581+
SyncCallAndThrowOnError(env, &req_wrap_sync, uv_fs_rmdir, *path);
15841582
FS_SYNC_TRACE_END(rmdir);
15851583
}
15861584
}

‎typings/internalBinding/fs.d.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,7 @@ declare namespace InternalFSBinding{
192192
functionrename(oldPath: string,newPath: string): void;
193193

194194
functionrmdir(path: string,req: FSReqCallback): void;
195-
functionrmdir(path: string,req: undefined,ctx: FSSyncContext): void;
195+
functionrmdir(path: string): void;
196196
functionrmdir(path: string,usePromises: typeofkUsePromises): Promise<void>;
197197

198198
functionstat(path: StringOrBuffer,useBigint: boolean,req: FSReqCallback<Float64Array|BigUint64Array>): void;

0 commit comments

Comments
(0)