Skip to content

Commit 6764f0c

Browse files
IlyasShabirichardlau
authored andcommitted
fs: improve error performance of readvSync
PR-URL: #50100 Refs: nodejs/performance#106 Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
1 parent a1feae2 commit 6764f0c

File tree

3 files changed

+67
-11
lines changed

3 files changed

+67
-11
lines changed

benchmark/fs/bench-readvSync.js

+58
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
const fs = require('fs');
5+
const assert = require('assert');
6+
const tmpdir = require('../../test/common/tmpdir');
7+
tmpdir.refresh();
8+
9+
const exptectedBuff = Buffer.from('Benchmark Data');
10+
const expectedLength = exptectedBuff.length;
11+
12+
const bufferArr = [Buffer.alloc(expectedLength)];
13+
14+
const filename = tmpdir.resolve('readv_sync.txt');
15+
fs.writeFileSync(filename, exptectedBuff);
16+
17+
const bench = common.createBenchmark(main, {
18+
type: ['valid', 'invalid'],
19+
n: [1e5],
20+
});
21+
22+
function main({ n, type }) {
23+
let fd;
24+
let result;
25+
26+
switch (type) {
27+
case 'valid':
28+
fd = fs.openSync(filename, 'r');
29+
30+
bench.start();
31+
for (let i = 0; i < n; i++) {
32+
result = fs.readvSync(fd, bufferArr, 0);
33+
}
34+
35+
bench.end(n);
36+
assert.strictEqual(result, expectedLength);
37+
fs.closeSync(fd);
38+
break;
39+
case 'invalid': {
40+
fd = 1 << 30;
41+
let hasError = false;
42+
bench.start();
43+
for (let i = 0; i < n; i++) {
44+
try {
45+
result = fs.readvSync(fd, bufferArr, 0);
46+
} catch {
47+
hasError = true;
48+
}
49+
}
50+
51+
bench.end(n);
52+
assert(hasError);
53+
break;
54+
}
55+
default:
56+
throw new Error('Invalid type');
57+
}
58+
}

lib/fs.js

+1-5
Original file line numberDiff line numberDiff line change
@@ -801,14 +801,10 @@ function readvSync(fd, buffers, position) {
801801
fd = getValidatedFd(fd);
802802
validateBufferArray(buffers);
803803

804-
const ctx = {};
805-
806804
if (typeof position !== 'number')
807805
position = null;
808806

809-
const result = binding.readBuffers(fd, buffers, position, undefined, ctx);
810-
handleErrorFromBinding(ctx);
811-
return result;
807+
return binding.readBuffers(fd, buffers, position);
812808
}
813809

814810
/**

src/node_file.cc

+8-6
Original file line numberDiff line numberDiff line change
@@ -2564,18 +2564,20 @@ static void ReadBuffers(const FunctionCallbackInfo<Value>& args) {
25642564
iovs[i] = uv_buf_init(Buffer::Data(buffer), Buffer::Length(buffer));
25652565
}
25662566

2567-
FSReqBase* req_wrap_async = GetReqWrap(args, 3);
2568-
if (req_wrap_async != nullptr) { // readBuffers(fd, buffers, pos, req)
2567+
if (argc > 3) { // readBuffers(fd, buffers, pos, req)
2568+
FSReqBase* req_wrap_async = GetReqWrap(args, 3);
25692569
FS_ASYNC_TRACE_BEGIN0(UV_FS_READ, req_wrap_async)
25702570
AsyncCall(env, req_wrap_async, args, "read", UTF8, AfterInteger,
25712571
uv_fs_read, fd, *iovs, iovs.length(), pos);
25722572
} else { // readBuffers(fd, buffers, undefined, ctx)
2573-
CHECK_EQ(argc, 5);
2574-
FSReqWrapSync req_wrap_sync;
2573+
FSReqWrapSync req_wrap_sync("read");
25752574
FS_SYNC_TRACE_BEGIN(read);
2576-
int bytesRead = SyncCall(env, /* ctx */ args[4], &req_wrap_sync, "read",
2577-
uv_fs_read, fd, *iovs, iovs.length(), pos);
2575+
int bytesRead = SyncCallAndThrowOnError(
2576+
env, &req_wrap_sync, uv_fs_read, fd, *iovs, iovs.length(), pos);
25782577
FS_SYNC_TRACE_END(read, "bytesRead", bytesRead);
2578+
if (is_uv_error(bytesRead)) {
2579+
return;
2580+
}
25792581
args.GetReturnValue().Set(bytesRead);
25802582
}
25812583
}

0 commit comments

Comments
 (0)