Skip to content

Commit d7132d9

Browse files
Jungku Leetargos
Jungku Lee
authored andcommitted
fs: improve error performance for fdatasyncSync
PR-URL: #49898 Refs: nodejs/performance#106 Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>
1 parent 55a03fe commit d7132d9

File tree

4 files changed

+51
-10
lines changed

4 files changed

+51
-10
lines changed

benchmark/fs/bench_fdatasyncSync.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 tmpfile = tmpdir.resolve(`.existing-file-${process.pid}`);
9+
fs.writeFileSync(tmpfile, 'this-is-for-a-benchmark', 'utf8');
10+
11+
const bench = common.createBenchmark(main, {
12+
type: ['existing', 'non-existing'],
13+
n: [1e4],
14+
});
15+
16+
function main({ n, type }) {
17+
let fd;
18+
19+
switch (type) {
20+
case 'existing':
21+
fd = fs.openSync(tmpfile, 'r', 0o666);
22+
break;
23+
case 'non-existing':
24+
fd = 1 << 30;
25+
break;
26+
default:
27+
new Error('Invalid type');
28+
}
29+
30+
bench.start();
31+
for (let i = 0; i < n; i++) {
32+
try {
33+
fs.fdatasyncSync(fd);
34+
} catch {
35+
// do nothing
36+
}
37+
}
38+
39+
bench.end(n);
40+
41+
if (type === 'existing') fs.closeSync(fd);
42+
}

lib/fs.js

+1-3
Original file line numberDiff line numberDiff line change
@@ -1303,9 +1303,7 @@ function fdatasync(fd, callback) {
13031303
*/
13041304
function fdatasyncSync(fd) {
13051305
fd = getValidatedFd(fd);
1306-
const ctx = {};
1307-
binding.fdatasync(fd, undefined, ctx);
1308-
handleErrorFromBinding(ctx);
1306+
return binding.fdatasync(fd);
13091307
}
13101308

13111309
/**

src/node_file.cc

+7-7
Original file line numberDiff line numberDiff line change
@@ -1536,21 +1536,21 @@ static void Fdatasync(const FunctionCallbackInfo<Value>& args) {
15361536
Environment* env = Environment::GetCurrent(args);
15371537

15381538
const int argc = args.Length();
1539-
CHECK_GE(argc, 2);
1539+
CHECK_GE(argc, 1);
15401540

15411541
CHECK(args[0]->IsInt32());
15421542
const int fd = args[0].As<Int32>()->Value();
15431543

1544-
FSReqBase* req_wrap_async = GetReqWrap(args, 1);
1545-
if (req_wrap_async != nullptr) {
1544+
if (argc > 1) { // fdatasync(fd, req)
1545+
FSReqBase* req_wrap_async = GetReqWrap(args, 1);
1546+
CHECK_NOT_NULL(req_wrap_async);
15461547
FS_ASYNC_TRACE_BEGIN0(UV_FS_FDATASYNC, req_wrap_async)
15471548
AsyncCall(env, req_wrap_async, args, "fdatasync", UTF8, AfterNoArgs,
15481549
uv_fs_fdatasync, fd);
1549-
} else {
1550-
CHECK_EQ(argc, 3);
1551-
FSReqWrapSync req_wrap_sync;
1550+
} else { // fdatasync(fd)
1551+
FSReqWrapSync req_wrap_sync("fdatasync");
15521552
FS_SYNC_TRACE_BEGIN(fdatasync);
1553-
SyncCall(env, args[2], &req_wrap_sync, "fdatasync", uv_fs_fdatasync, fd);
1553+
SyncCallAndThrowOnError(env, &req_wrap_sync, uv_fs_fdatasync, fd);
15541554
FS_SYNC_TRACE_END(fdatasync);
15551555
}
15561556
}

typings/internalBinding/fs.d.ts

+1
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ declare namespace InternalFSBinding {
8686
function fdatasync(fd: number, req: FSReqCallback): void;
8787
function fdatasync(fd: number, req: undefined, ctx: FSSyncContext): void;
8888
function fdatasync(fd: number, usePromises: typeof kUsePromises): Promise<void>;
89+
function fdatasync(fd: number): void;
8990

9091
function fstat(fd: number, useBigint: boolean, req: FSReqCallback<Float64Array | BigUint64Array>): void;
9192
function fstat(fd: number, useBigint: true, req: FSReqCallback<BigUint64Array>): void;

0 commit comments

Comments
 (0)