Skip to content

Commit 918060c

Browse files
author
pluris
committed
fs: improve error performance for writeSync
1 parent 342ddb0 commit 918060c

3 files changed

Lines changed: 77 additions & 19 deletions

File tree

benchmark/fs/bench-writeSync.js

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
const fs = require('fs');
5+
const assert = require('assert');
6+
const tmpdir = require('../../test/common/tmpdir');
7+
tmpdir.refresh();
8+
9+
const path = tmpdir.resolve(`new-file-${process.pid}`);
10+
const parameters = [Buffer.from('Benchmark data'),
11+
0,
12+
Buffer.byteLength('Benchmark data')];
13+
const bench = common.createBenchmark(main, {
14+
type: ['valid', 'invalid'],
15+
n: [1e5],
16+
});
17+
18+
function main({ n, type }) {
19+
let fd;
20+
let result;
21+
22+
switch (type) {
23+
case 'valid':
24+
fd = fs.openSync(path, 'w');
25+
26+
bench.start();
27+
for (let i = 0; i < n; i++) {
28+
result = fs.writeSync(fd, ...parameters);
29+
}
30+
31+
bench.end(n);
32+
assert(result);
33+
fs.closeSync(fd);
34+
break;
35+
case 'invalid': {
36+
fd = 1 << 30;
37+
let hasError = false;
38+
bench.start();
39+
for (let i = 0; i < n; i++) {
40+
try {
41+
result = fs.writeSync(fd, ...parameters);
42+
} catch {
43+
hasError = true;
44+
}
45+
}
46+
47+
bench.end(n);
48+
assert(hasError);
49+
break;
50+
}
51+
default:
52+
throw new Error('Invalid type');
53+
}
54+
}

lib/fs.js

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -896,7 +896,6 @@ ObjectDefineProperty(write, kCustomPromisifyArgsSymbol,
896896
*/
897897
function writeSync(fd, buffer, offsetOrOptions, length, position) {
898898
fd = getValidatedFd(fd);
899-
const ctx = {};
900899
let result;
901900

902901
let offset = offsetOrOptions;
@@ -918,18 +917,15 @@ function writeSync(fd, buffer, offsetOrOptions, length, position) {
918917
if (typeof length !== 'number')
919918
length = buffer.byteLength - offset;
920919
validateOffsetLengthWrite(offset, length, buffer.byteLength);
921-
result = binding.writeBuffer(fd, buffer, offset, length, position,
922-
undefined, ctx);
920+
result = binding.writeBuffer(fd, buffer, offset, length, position);
923921
} else {
924922
validateStringAfterArrayBufferView(buffer, 'buffer');
925923
validateEncoding(buffer, length);
926924

927925
if (offset === undefined)
928926
offset = null;
929-
result = binding.writeString(fd, buffer, offset, length,
930-
undefined, ctx);
927+
result = binding.writeString(fd, buffer, offset, length);
931928
}
932-
handleErrorFromBinding(ctx);
933929
return result;
934930
}
935931

src/node_file.cc

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2031,7 +2031,7 @@ static void WriteBuffer(const FunctionCallbackInfo<Value>& args) {
20312031
Environment* env = Environment::GetCurrent(args);
20322032

20332033
const int argc = args.Length();
2034-
CHECK_GE(argc, 4);
2034+
CHECK_GE(argc, 5);
20352035

20362036
CHECK(args[0]->IsInt32());
20372037
const int fd = args[0].As<Int32>()->Value();
@@ -2058,18 +2058,22 @@ static void WriteBuffer(const FunctionCallbackInfo<Value>& args) {
20582058
char* buf = buffer_data + off;
20592059
uv_buf_t uvbuf = uv_buf_init(buf, len);
20602060

2061-
FSReqBase* req_wrap_async = GetReqWrap(args, 5);
2062-
if (req_wrap_async != nullptr) { // write(fd, buffer, off, len, pos, req)
2061+
if (argc > 5) { // write(fd, buffer, off, len, pos, req)
2062+
FSReqBase* req_wrap_async = GetReqWrap(args, 5);
20632063
FS_ASYNC_TRACE_BEGIN0(UV_FS_WRITE, req_wrap_async)
20642064
AsyncCall(env, req_wrap_async, args, "write", UTF8, AfterInteger,
20652065
uv_fs_write, fd, &uvbuf, 1, pos);
2066-
} else { // write(fd, buffer, off, len, pos, undefined, ctx)
2067-
CHECK_EQ(argc, 7);
2068-
FSReqWrapSync req_wrap_sync;
2066+
} else { // write(fd, buffer, off, len, pos)
2067+
FSReqWrapSync req_wrap_sync("write");
20692068
FS_SYNC_TRACE_BEGIN(write);
2070-
int bytesWritten = SyncCall(env, args[6], &req_wrap_sync, "write",
2071-
uv_fs_write, fd, &uvbuf, 1, pos);
2069+
int bytesWritten = SyncCallAndThrowOnError(
2070+
env, &req_wrap_sync, uv_fs_write, fd, &uvbuf, 1, pos);
20722071
FS_SYNC_TRACE_END(write, "bytesWritten", bytesWritten);
2072+
2073+
if (is_uv_error(bytesWritten)) {
2074+
return;
2075+
}
2076+
20732077
args.GetReturnValue().Set(bytesWritten);
20742078
}
20752079
}
@@ -2206,9 +2210,8 @@ static void WriteString(const FunctionCallbackInfo<Value>& args) {
22062210
} else {
22072211
req_wrap_async->SetReturnValue(args);
22082212
}
2209-
} else { // write(fd, string, pos, enc, undefined, ctx)
2210-
CHECK_EQ(argc, 6);
2211-
FSReqWrapSync req_wrap_sync;
2213+
} else { // write(fd, string, pos, enc)
2214+
FSReqWrapSync req_wrap_sync("write");
22122215
FSReqBase::FSReqBuffer stack_buffer;
22132216
if (buf == nullptr) {
22142217
if (!StringBytes::StorageSize(isolate, value, enc).To(&len))
@@ -2223,9 +2226,14 @@ static void WriteString(const FunctionCallbackInfo<Value>& args) {
22232226
}
22242227
uv_buf_t uvbuf = uv_buf_init(buf, len);
22252228
FS_SYNC_TRACE_BEGIN(write);
2226-
int bytesWritten = SyncCall(env, args[5], &req_wrap_sync, "write",
2227-
uv_fs_write, fd, &uvbuf, 1, pos);
2229+
int bytesWritten = SyncCallAndThrowOnError(
2230+
env, &req_wrap_sync, uv_fs_write, fd, &uvbuf, 1, pos);
22282231
FS_SYNC_TRACE_END(write, "bytesWritten", bytesWritten);
2232+
2233+
if (is_uv_error(bytesWritten)) {
2234+
return;
2235+
}
2236+
22292237
args.GetReturnValue().Set(bytesWritten);
22302238
}
22312239
}

0 commit comments

Comments
 (0)