Skip to content

Commit aa3209b

Browse files
CanadaHonkUlisesGascon
authored andcommitted
fs: add c++ fast path for writeFileSync utf8
PR-URL: #49884 Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io> Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com> Reviewed-By: Stephen Belanger <admin@stephenbelanger.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 4cf155c commit aa3209b

File tree

5 files changed

+156
-0
lines changed

5 files changed

+156
-0
lines changed

benchmark/fs/bench-writeFileSync.js

+43
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
'use strict';
2+
3+
const common = require('../common.js');
4+
const fs = require('fs');
5+
const tmpdir = require('../../test/common/tmpdir');
6+
tmpdir.refresh();
7+
8+
// Some variants are commented out as they do not show a change and just slow
9+
const bench = common.createBenchmark(main, {
10+
encoding: ['utf8'],
11+
useFd: ['true', 'false'],
12+
length: [1024, 102400, 1024 * 1024],
13+
14+
// useBuffer: ['true', 'false'],
15+
useBuffer: ['false'],
16+
17+
// func: ['appendFile', 'writeFile'],
18+
func: ['writeFile'],
19+
20+
n: [1e3],
21+
});
22+
23+
function main({ n, func, encoding, length, useFd, useBuffer }) {
24+
tmpdir.refresh();
25+
const enc = encoding === 'undefined' ? undefined : encoding;
26+
const path = tmpdir.resolve(`.writefilesync-file-${Date.now()}`);
27+
28+
useFd = useFd === 'true';
29+
const file = useFd ? fs.openSync(path, 'w') : path;
30+
31+
let data = 'a'.repeat(length);
32+
if (useBuffer === 'true') data = Buffer.from(data, encoding);
33+
34+
const fn = fs[func + 'Sync'];
35+
36+
bench.start();
37+
for (let i = 0; i < n; ++i) {
38+
fn(file, data, enc);
39+
}
40+
bench.end(n);
41+
42+
if (useFd) fs.closeSync(file);
43+
}

lib/fs.js

+13
Original file line numberDiff line numberDiff line change
@@ -2341,6 +2341,19 @@ function writeFileSync(path, data, options) {
23412341

23422342
validateBoolean(flush, 'options.flush');
23432343

2344+
// C++ fast path for string data and UTF8 encoding
2345+
if (typeof data === 'string' && (options.encoding === 'utf8' || options.encoding === 'utf-8')) {
2346+
if (!isInt32(path)) {
2347+
path = pathModule.toNamespacedPath(getValidatedPath(path));
2348+
}
2349+
2350+
return binding.writeFileUtf8(
2351+
path, data,
2352+
stringToFlags(options.flag),
2353+
parseFileMode(options.mode, 'mode', 0o666),
2354+
);
2355+
}
2356+
23442357
if (!isArrayBufferView(data)) {
23452358
validateStringAfterArrayBufferView(data, 'data');
23462359
data = Buffer.from(data, options.encoding || 'utf8');

src/node_file.cc

+80
Original file line numberDiff line numberDiff line change
@@ -2327,6 +2327,84 @@ static void WriteString(const FunctionCallbackInfo<Value>& args) {
23272327
}
23282328
}
23292329

2330+
static void WriteFileUtf8(const FunctionCallbackInfo<Value>& args) {
2331+
// Fast C++ path for fs.writeFileSync(path, data) with utf8 encoding
2332+
// (file, data, options.flag, options.mode)
2333+
2334+
Environment* env = Environment::GetCurrent(args);
2335+
auto isolate = env->isolate();
2336+
2337+
CHECK_EQ(args.Length(), 4);
2338+
2339+
BufferValue value(isolate, args[1]);
2340+
CHECK_NOT_NULL(*value);
2341+
2342+
CHECK(args[2]->IsInt32());
2343+
const int flags = args[2].As<Int32>()->Value();
2344+
2345+
CHECK(args[3]->IsInt32());
2346+
const int mode = args[3].As<Int32>()->Value();
2347+
2348+
uv_file file;
2349+
2350+
bool is_fd = args[0]->IsInt32();
2351+
2352+
// Check for file descriptor
2353+
if (is_fd) {
2354+
file = args[0].As<Int32>()->Value();
2355+
} else {
2356+
BufferValue path(isolate, args[0]);
2357+
CHECK_NOT_NULL(*path);
2358+
if (CheckOpenPermissions(env, path, flags).IsNothing()) return;
2359+
2360+
FSReqWrapSync req_open("open", *path);
2361+
2362+
FS_SYNC_TRACE_BEGIN(open);
2363+
file =
2364+
SyncCallAndThrowOnError(env, &req_open, uv_fs_open, *path, flags, mode);
2365+
FS_SYNC_TRACE_END(open);
2366+
2367+
if (is_uv_error(file)) {
2368+
return;
2369+
}
2370+
}
2371+
2372+
int bytesWritten = 0;
2373+
uint32_t offset = 0;
2374+
2375+
const size_t length = value.length();
2376+
uv_buf_t uvbuf = uv_buf_init(value.out(), length);
2377+
2378+
FS_SYNC_TRACE_BEGIN(write);
2379+
while (offset < length) {
2380+
FSReqWrapSync req_write("write");
2381+
bytesWritten = SyncCallAndThrowOnError(
2382+
env, &req_write, uv_fs_write, file, &uvbuf, 1, -1);
2383+
2384+
// Write errored out
2385+
if (bytesWritten < 0) {
2386+
break;
2387+
}
2388+
2389+
offset += bytesWritten;
2390+
DCHECK_LE(offset, length);
2391+
uvbuf.base += bytesWritten;
2392+
uvbuf.len -= bytesWritten;
2393+
}
2394+
FS_SYNC_TRACE_END(write);
2395+
2396+
if (!is_fd) {
2397+
FSReqWrapSync req_close("close");
2398+
2399+
FS_SYNC_TRACE_BEGIN(close);
2400+
int result = SyncCallAndThrowOnError(env, &req_close, uv_fs_close, file);
2401+
FS_SYNC_TRACE_END(close);
2402+
2403+
if (is_uv_error(result)) {
2404+
return;
2405+
}
2406+
}
2407+
}
23302408

23312409
/*
23322410
* Wrapper for read(2).
@@ -3169,6 +3247,7 @@ static void CreatePerIsolateProperties(IsolateData* isolate_data,
31693247
SetMethod(isolate, target, "writeBuffer", WriteBuffer);
31703248
SetMethod(isolate, target, "writeBuffers", WriteBuffers);
31713249
SetMethod(isolate, target, "writeString", WriteString);
3250+
SetMethod(isolate, target, "writeFileUtf8", WriteFileUtf8);
31723251
SetMethod(isolate, target, "realpath", RealPath);
31733252
SetMethod(isolate, target, "copyFile", CopyFile);
31743253

@@ -3289,6 +3368,7 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
32893368
registry->Register(WriteBuffer);
32903369
registry->Register(WriteBuffers);
32913370
registry->Register(WriteString);
3371+
registry->Register(WriteFileUtf8);
32923372
registry->Register(RealPath);
32933373
registry->Register(CopyFile);
32943374

test/parallel/test-fs-sync-fd-leak.js

+17
Original file line numberDiff line numberDiff line change
@@ -41,17 +41,34 @@ fs.writeSync = function() {
4141
throw new Error('BAM');
4242
};
4343

44+
// Internal fast paths are pure C++, can't error inside write
45+
internalBinding('fs').writeFileUtf8 = function() {
46+
// Fake close
47+
close_called++;
48+
throw new Error('BAM');
49+
};
50+
4451
internalBinding('fs').fstat = function() {
4552
throw new Error('EBADF: bad file descriptor, fstat');
4653
};
4754

4855
let close_called = 0;
4956
ensureThrows(function() {
57+
// Fast path: writeFileSync utf8
5058
fs.writeFileSync('dummy', 'xxx');
5159
}, 'BAM');
5260
ensureThrows(function() {
61+
// Non-fast path
62+
fs.writeFileSync('dummy', 'xxx', { encoding: 'base64' });
63+
}, 'BAM');
64+
ensureThrows(function() {
65+
// Fast path: writeFileSync utf8
5366
fs.appendFileSync('dummy', 'xxx');
5467
}, 'BAM');
68+
ensureThrows(function() {
69+
// Non-fast path
70+
fs.appendFileSync('dummy', 'xxx', { encoding: 'base64' });
71+
}, 'BAM');
5572

5673
function ensureThrows(cb, message) {
5774
let got_exception = false;

typings/internalBinding/fs.d.ts

+3
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,9 @@ declare namespace InternalFSBinding {
231231
function writeString(fd: number, value: string, pos: unknown, encoding: unknown, usePromises: typeof kUsePromises): Promise<number>;
232232

233233
function getFormatOfExtensionlessFile(url: string): ConstantsBinding['fs'];
234+
235+
function writeFileUtf8(path: string, data: string, flag: number, mode: number): void;
236+
function writeFileUtf8(fd: number, data: string, flag: number, mode: number): void;
234237
}
235238

236239
export interface FsBinding {

0 commit comments

Comments
 (0)