From 8f8be72c19abecfec76b59b8fe8fa93ab485fba4 Mon Sep 17 00:00:00 2001 From: pluris Date: Tue, 24 Oct 2023 12:41:35 +0900 Subject: [PATCH] fs: improve error performance for `writeSync` --- benchmark/fs/bench-writeSync.js | 54 +++++++++++++++++++++++++++++++++ lib/fs.js | 9 ++---- lib/internal/net.js | 11 +++---- src/node_file.cc | 30 ++++++++++-------- typings/internalBinding/fs.d.ts | 4 +-- 5 files changed, 79 insertions(+), 29 deletions(-) create mode 100644 benchmark/fs/bench-writeSync.js diff --git a/benchmark/fs/bench-writeSync.js b/benchmark/fs/bench-writeSync.js new file mode 100644 index 00000000000000..4edef0babca724 --- /dev/null +++ b/benchmark/fs/bench-writeSync.js @@ -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'); + } +} diff --git a/lib/fs.js b/lib/fs.js index 8ddbf18cdd4865..fcf0477252e86d 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -107,7 +107,6 @@ const { getValidatedFd, getValidatedPath, getValidMode, - handleErrorFromBinding, possiblyTransformPath, preprocessSymlinkDestination, Stats, @@ -898,7 +897,6 @@ ObjectDefineProperty(write, kCustomPromisifyArgsSymbol, */ function writeSync(fd, buffer, offsetOrOptions, length, position) { fd = getValidatedFd(fd); - const ctx = {}; let result; let offset = offsetOrOptions; @@ -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; } diff --git a/lib/internal/net.js b/lib/internal/net.js index 0f2fc5dfff5ea8..f77dc107718605 100644 --- a/lib/internal/net.js +++ b/lib/internal/net.js @@ -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])'; @@ -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(); }; diff --git a/src/node_file.cc b/src/node_file.cc index a3e80898cde6b6..0661e85987d980 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -2030,7 +2030,7 @@ static void WriteBuffer(const FunctionCallbackInfo& 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()->Value(); @@ -2057,18 +2057,20 @@ static void WriteBuffer(const FunctionCallbackInfo& 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 > 5) { // 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); } } @@ -2205,9 +2207,8 @@ static void WriteString(const FunctionCallbackInfo& 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)) @@ -2222,9 +2223,12 @@ static void WriteString(const FunctionCallbackInfo& 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); } } diff --git a/typings/internalBinding/fs.d.ts b/typings/internalBinding/fs.d.ts index 5a4741e99fa727..fb4669a047da54 100644 --- a/typings/internalBinding/fs.d.ts +++ b/typings/internalBinding/fs.d.ts @@ -218,7 +218,7 @@ declare namespace InternalFSBinding { function utimes(path: string, atime: number, mtime: number, usePromises: typeof kUsePromises): Promise; function writeBuffer(fd: number, buffer: ArrayBufferView, offset: number, length: number, position: number | null, req: FSReqCallback): 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; function writeBuffers(fd: number, buffers: ArrayBufferView[], position: number, req: FSReqCallback): void; @@ -226,7 +226,7 @@ declare namespace InternalFSBinding { function writeBuffers(fd: number, buffers: ArrayBufferView[], position: number, usePromises: typeof kUsePromises): Promise; function writeString(fd: number, value: string, pos: unknown, encoding: unknown, req: FSReqCallback): 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; function getFormatOfExtensionlessFile(url: string): ConstantsBinding['fs'];