From 352378aafd77b362729c63efc60dfa11ed02042e Mon Sep 17 00:00:00 2001 From: Shubham Pandey Date: Sun, 26 Nov 2023 23:40:56 +0530 Subject: [PATCH] fs: improve error performance of fs.writeSync --- benchmark/fs/bench-writeFileSync.js | 20 +++++++++++++++++ benchmark/fs/bench-writeSync.js | 34 +++++++++++++++++++++++++++++ lib/fs.js | 7 ++---- src/node_file.cc | 30 ++++++++++++++----------- 4 files changed, 73 insertions(+), 18 deletions(-) create mode 100644 benchmark/fs/bench-writeFileSync.js create mode 100644 benchmark/fs/bench-writeSync.js diff --git a/benchmark/fs/bench-writeFileSync.js b/benchmark/fs/bench-writeFileSync.js new file mode 100644 index 00000000000000..3e17d2e5925bc9 --- /dev/null +++ b/benchmark/fs/bench-writeFileSync.js @@ -0,0 +1,20 @@ +'use strict'; + +const common = require('../common'); +const fs = require('fs'); +const tmpdir = require('../../test/common/tmpdir'); +tmpdir.refresh(); + +const bench = common.createBenchmark(main, { + n: [1e4], +}); + +function main({ n, type }) { + const path = tmpdir.resolve(`new-file-${process.pid}`); + + bench.start(); + for (let i = 0; i < n; i++) { + fs.writeFileSync(path, 'Benchmark data.'); + } + bench.end(n); +} diff --git a/benchmark/fs/bench-writeSync.js b/benchmark/fs/bench-writeSync.js new file mode 100644 index 00000000000000..686e887775d19e --- /dev/null +++ b/benchmark/fs/bench-writeSync.js @@ -0,0 +1,34 @@ +'use strict'; + +const common = require('../common'); +const fs = require('fs'); +const tmpdir = require('../../test/common/tmpdir'); +tmpdir.refresh(); + +const path = tmpdir.resolve(`new-file-${process.pid}`); +fs.writeFileSync(path, 'Some content.'); + +const bench = common.createBenchmark(main, { + type: ['valid', 'invalid'], + dataType: ['string', 'buffer'], + n: [1e5], +}); + +const stringData = 'Benchmark data.'; +const bufferData = Buffer.from(stringData); + +function main({ n, type, dataType }) { + const fd = type === 'valid' ? fs.openSync(path, 'r+') : 1 << 30; + const data = dataType === 'string' ? stringData : bufferData; + + bench.start(); + for (let i = 0; i < n; i++) { + try { + fs.writeSync(fd, data); + } catch { + // Continue regardless of error. + } + } + bench.end(n); + if (type === 'valid') fs.closeSync(fd); +} diff --git a/lib/fs.js b/lib/fs.js index 62dedb1566a4af..e9fc037579f7f7 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -922,18 +922,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/src/node_file.cc b/src/node_file.cc index 502a9ab712072a..2b07a85cdc089e 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -2033,7 +2033,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(); @@ -2060,18 +2060,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); } } @@ -2208,9 +2210,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)) @@ -2225,9 +2226,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); } }