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 Feb 28, 2024
1 parent 8e7da60 commit 975ec4a
Show file tree
Hide file tree
Showing 5 changed files with 79 additions and 27 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,
preprocessSymlinkDestination,
Stats,
getStatFsFromBinding,
Expand Down Expand Up @@ -897,7 +896,6 @@ ObjectDefineProperty(write, kCustomPromisifyArgsSymbol,
*/
function writeSync(fd, buffer, offsetOrOptions, length, position) {
fd = getValidatedFd(fd);
const ctx = {};
let result;

let offset = offsetOrOptions;
Expand All @@ -919,18 +917,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
11 changes: 4 additions & 7 deletions lib/internal/net.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ const {

const Buffer = require('buffer').Buffer;
const { writeBuffer } = internalBinding('fs');
const errors = require('internal/errors');

// IPv4 Segment
const v4Seg = '(?:25[0-5]|2[0-4][0-9]|1[0-9][0-9]|[1-9][0-9]|[0-9])';
Expand Down Expand Up @@ -55,12 +54,10 @@ function makeSyncWrite(fd) {

this._handle.bytesWritten += chunk.length;

const ctx = {};
writeBuffer(fd, chunk, 0, chunk.length, null, undefined, ctx);
if (ctx.errno !== undefined) {
const ex = new errors.UVException(ctx);
ex.errno = ctx.errno;
return cb(ex);
try {
writeBuffer(fd, chunk, 0, chunk.length, null);
} catch (e) {
return cb(e);
}
cb();
};
Expand Down
28 changes: 17 additions & 11 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 Down Expand Up @@ -2062,13 +2062,16 @@ static void WriteBuffer(const FunctionCallbackInfo<Value>& args) {
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)
CHECK_EQ(argc, 5);
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 +2208,9 @@ 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)
CHECK_EQ(argc, 4);
FSReqWrapSync req_wrap_sync("write");
FSReqBase::FSReqBuffer stack_buffer;
if (buf == nullptr) {
if (!StringBytes::StorageSize(isolate, value, enc).To(&len))
Expand All @@ -2222,9 +2225,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
4 changes: 2 additions & 2 deletions typings/internalBinding/fs.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -218,15 +218,15 @@ declare namespace InternalFSBinding {
function utimes(path: string, atime: number, mtime: number, usePromises: typeof kUsePromises): Promise<void>;

function writeBuffer(fd: number, buffer: ArrayBufferView, offset: number, length: number, position: number | null, req: FSReqCallback<number>): void;
function writeBuffer(fd: number, buffer: ArrayBufferView, offset: number, length: number, position: number | null, req: undefined, ctx: FSSyncContext): number;
function writeBuffer(fd: number, buffer: ArrayBufferView, offset: number, length: number, position: number | null): number;
function writeBuffer(fd: number, buffer: ArrayBufferView, offset: number, length: number, position: number | null, usePromises: typeof kUsePromises): Promise<number>;

function writeBuffers(fd: number, buffers: ArrayBufferView[], position: number, req: FSReqCallback<number>): void;
function writeBuffers(fd: number, buffers: ArrayBufferView[], position: number, req: undefined, ctx: FSSyncContext): number;
function writeBuffers(fd: number, buffers: ArrayBufferView[], position: number, usePromises: typeof kUsePromises): Promise<number>;

function writeString(fd: number, value: string, pos: unknown, encoding: unknown, req: FSReqCallback<number>): void;
function writeString(fd: number, value: string, pos: unknown, encoding: unknown, req: undefined, ctx: FSSyncContext): number;
function writeString(fd: number, value: string, pos: unknown, encoding: unknown): number;
function writeString(fd: number, value: string, pos: unknown, encoding: unknown, usePromises: typeof kUsePromises): Promise<number>;

function getFormatOfExtensionlessFile(url: string): ConstantsBinding['fs'];
Expand Down

0 comments on commit 975ec4a

Please sign in to comment.