Skip to content

Commit

Permalink
fs: improve error performance of readvSync
Browse files Browse the repository at this point in the history
PR-URL: #50100
Refs: nodejs/performance#106
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
  • Loading branch information
IlyasShabi authored Dec 5, 2023
1 parent 4957994 commit 27b2ce5
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 11 deletions.
58 changes: 58 additions & 0 deletions benchmark/fs/bench-readvSync.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
'use strict';

const common = require('../common');
const fs = require('fs');
const assert = require('assert');
const tmpdir = require('../../test/common/tmpdir');
tmpdir.refresh();

const exptectedBuff = Buffer.from('Benchmark Data');
const expectedLength = exptectedBuff.length;

const bufferArr = [Buffer.alloc(expectedLength)];

const filename = tmpdir.resolve('readv_sync.txt');
fs.writeFileSync(filename, exptectedBuff);

const bench = common.createBenchmark(main, {
type: ['valid', 'invalid'],
n: [1e5],
});

function main({ n, type }) {
let fd;
let result;

switch (type) {
case 'valid':
fd = fs.openSync(filename, 'r');

bench.start();
for (let i = 0; i < n; i++) {
result = fs.readvSync(fd, bufferArr, 0);
}

bench.end(n);
assert.strictEqual(result, expectedLength);
fs.closeSync(fd);
break;
case 'invalid': {
fd = 1 << 30;
let hasError = false;
bench.start();
for (let i = 0; i < n; i++) {
try {
result = fs.readvSync(fd, bufferArr, 0);
} catch {
hasError = true;
}
}

bench.end(n);
assert(hasError);
break;
}
default:
throw new Error('Invalid type');
}
}
6 changes: 1 addition & 5 deletions lib/fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -801,14 +801,10 @@ function readvSync(fd, buffers, position) {
fd = getValidatedFd(fd);
validateBufferArray(buffers);

const ctx = {};

if (typeof position !== 'number')
position = null;

const result = binding.readBuffers(fd, buffers, position, undefined, ctx);
handleErrorFromBinding(ctx);
return result;
return binding.readBuffers(fd, buffers, position);
}

/**
Expand Down
14 changes: 8 additions & 6 deletions src/node_file.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2467,18 +2467,20 @@ static void ReadBuffers(const FunctionCallbackInfo<Value>& args) {
iovs[i] = uv_buf_init(Buffer::Data(buffer), Buffer::Length(buffer));
}

FSReqBase* req_wrap_async = GetReqWrap(args, 3);
if (req_wrap_async != nullptr) { // readBuffers(fd, buffers, pos, req)
if (argc > 3) { // readBuffers(fd, buffers, pos, req)
FSReqBase* req_wrap_async = GetReqWrap(args, 3);
FS_ASYNC_TRACE_BEGIN0(UV_FS_READ, req_wrap_async)
AsyncCall(env, req_wrap_async, args, "read", UTF8, AfterInteger,
uv_fs_read, fd, *iovs, iovs.length(), pos);
} else { // readBuffers(fd, buffers, undefined, ctx)
CHECK_EQ(argc, 5);
FSReqWrapSync req_wrap_sync;
FSReqWrapSync req_wrap_sync("read");
FS_SYNC_TRACE_BEGIN(read);
int bytesRead = SyncCall(env, /* ctx */ args[4], &req_wrap_sync, "read",
uv_fs_read, fd, *iovs, iovs.length(), pos);
int bytesRead = SyncCallAndThrowOnError(
env, &req_wrap_sync, uv_fs_read, fd, *iovs, iovs.length(), pos);
FS_SYNC_TRACE_END(read, "bytesRead", bytesRead);
if (is_uv_error(bytesRead)) {
return;
}
args.GetReturnValue().Set(bytesRead);
}
}
Expand Down

0 comments on commit 27b2ce5

Please sign in to comment.