Skip to content

Commit

Permalink
fs: improve error performance for writeSync
Browse files Browse the repository at this point in the history
  • Loading branch information
pluris committed Dec 25, 2023
1 parent 6a1abd2 commit 1055136
Show file tree
Hide file tree
Showing 3 changed files with 73 additions and 20 deletions.
54 changes: 54 additions & 0 deletions benchmark/fs/bench-writeSync.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
'use strict';

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

const path = tmpdir.resolve(`new-file-${process.pid}`);
const parameters = [Buffer.from('Benchmark data'),
0,
Buffer.byteLength('Benchmark data')];
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(path, 'w');

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

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

bench.end(n);
assert(hasError);
break;
}
default:
throw new Error('Invalid type');
}
}
9 changes: 2 additions & 7 deletions lib/fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,6 @@ const {
getValidatedFd,
getValidatedPath,
getValidMode,
handleErrorFromBinding,
possiblyTransformPath,
preprocessSymlinkDestination,
Stats,
Expand Down Expand Up @@ -898,7 +897,6 @@ ObjectDefineProperty(write, kCustomPromisifyArgsSymbol,
*/
function writeSync(fd, buffer, offsetOrOptions, length, position) {
fd = getValidatedFd(fd);
const ctx = {};
let result;

let offset = offsetOrOptions;
Expand All @@ -920,18 +918,15 @@ function writeSync(fd, buffer, offsetOrOptions, length, position) {
if (typeof length !== 'number')
length = buffer.byteLength - offset;
validateOffsetLengthWrite(offset, length, buffer.byteLength);
result = binding.writeBuffer(fd, buffer, offset, length, position,
undefined, ctx);
result = binding.writeBuffer(fd, buffer, offset, length, position);
} else {
validateStringAfterArrayBufferView(buffer, 'buffer');
validateEncoding(buffer, length);

if (offset === undefined)
offset = null;
result = binding.writeString(fd, buffer, offset, length,
undefined, ctx);
result = binding.writeString(fd, buffer, offset, length);
}
handleErrorFromBinding(ctx);
return result;
}

Expand Down
30 changes: 17 additions & 13 deletions src/node_file.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2030,7 +2030,7 @@ static void WriteBuffer(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);

const int argc = args.Length();
CHECK_GE(argc, 4);
CHECK_GE(argc, 5);

CHECK(args[0]->IsInt32());
const int fd = args[0].As<Int32>()->Value();
Expand All @@ -2057,18 +2057,20 @@ static void WriteBuffer(const FunctionCallbackInfo<Value>& args) {
char* buf = buffer_data + off;
uv_buf_t uvbuf = uv_buf_init(buf, len);

FSReqBase* req_wrap_async = GetReqWrap(args, 5);
if (req_wrap_async != nullptr) { // write(fd, buffer, off, len, pos, req)
if (argc == 6) { // write(fd, buffer, off, len, pos, req)
FSReqBase* req_wrap_async = GetReqWrap(args, 5);
FS_ASYNC_TRACE_BEGIN0(UV_FS_WRITE, req_wrap_async)
AsyncCall(env, req_wrap_async, args, "write", UTF8, AfterInteger,
uv_fs_write, fd, &uvbuf, 1, pos);
} else { // write(fd, buffer, off, len, pos, undefined, ctx)
CHECK_EQ(argc, 7);
FSReqWrapSync req_wrap_sync;
} else { // write(fd, buffer, off, len, pos)
FSReqWrapSync req_wrap_sync("write");
FS_SYNC_TRACE_BEGIN(write);
int bytesWritten = SyncCall(env, args[6], &req_wrap_sync, "write",
uv_fs_write, fd, &uvbuf, 1, pos);
int bytesWritten = SyncCallAndThrowOnError(
env, &req_wrap_sync, uv_fs_write, fd, &uvbuf, 1, pos);
FS_SYNC_TRACE_END(write, "bytesWritten", bytesWritten);
if (is_uv_error(bytesWritten)) {
return;
}
args.GetReturnValue().Set(bytesWritten);
}
}
Expand Down Expand Up @@ -2205,9 +2207,8 @@ static void WriteString(const FunctionCallbackInfo<Value>& args) {
} else {
req_wrap_async->SetReturnValue(args);
}
} else { // write(fd, string, pos, enc, undefined, ctx)
CHECK_EQ(argc, 6);
FSReqWrapSync req_wrap_sync;
} else { // write(fd, string, pos, enc)
FSReqWrapSync req_wrap_sync("write");
FSReqBase::FSReqBuffer stack_buffer;
if (buf == nullptr) {
if (!StringBytes::StorageSize(isolate, value, enc).To(&len))
Expand All @@ -2222,9 +2223,12 @@ static void WriteString(const FunctionCallbackInfo<Value>& args) {
}
uv_buf_t uvbuf = uv_buf_init(buf, len);
FS_SYNC_TRACE_BEGIN(write);
int bytesWritten = SyncCall(env, args[5], &req_wrap_sync, "write",
uv_fs_write, fd, &uvbuf, 1, pos);
int bytesWritten = SyncCallAndThrowOnError(
env, &req_wrap_sync, uv_fs_write, fd, &uvbuf, 1, pos);
FS_SYNC_TRACE_END(write, "bytesWritten", bytesWritten);
if (is_uv_error(bytesWritten)) {
return;
}
args.GetReturnValue().Set(bytesWritten);
}
}
Expand Down

0 comments on commit 1055136

Please sign in to comment.