Skip to content

Commit e59268a

Browse files
CanadaHonkRafaelGSS
authored andcommitted
fs: add c++ fast path for writeFileSync utf8
PR-URL: #49884 Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Santiago Gimeno <[email protected]> Reviewed-By: Stephen Belanger <[email protected]> Reviewed-By: James M Snell <[email protected]>
1 parent 483200f commit e59268a

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
@@ -2343,6 +2343,19 @@ function writeFileSync(path, data, options) {
23432343

23442344
validateBoolean(flush, 'options.flush');
23452345

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

src/node_file.cc

+80
Original file line numberDiff line numberDiff line change
@@ -2292,6 +2292,84 @@ static void WriteString(const FunctionCallbackInfo<Value>& args) {
22922292
}
22932293
}
22942294

2295+
static void WriteFileUtf8(const FunctionCallbackInfo<Value>& args) {
2296+
// Fast C++ path for fs.writeFileSync(path, data) with utf8 encoding
2297+
// (file, data, options.flag, options.mode)
2298+
2299+
Environment* env = Environment::GetCurrent(args);
2300+
auto isolate = env->isolate();
2301+
2302+
CHECK_EQ(args.Length(), 4);
2303+
2304+
BufferValue value(isolate, args[1]);
2305+
CHECK_NOT_NULL(*value);
2306+
2307+
CHECK(args[2]->IsInt32());
2308+
const int flags = args[2].As<Int32>()->Value();
2309+
2310+
CHECK(args[3]->IsInt32());
2311+
const int mode = args[3].As<Int32>()->Value();
2312+
2313+
uv_file file;
2314+
2315+
bool is_fd = args[0]->IsInt32();
2316+
2317+
// Check for file descriptor
2318+
if (is_fd) {
2319+
file = args[0].As<Int32>()->Value();
2320+
} else {
2321+
BufferValue path(isolate, args[0]);
2322+
CHECK_NOT_NULL(*path);
2323+
if (CheckOpenPermissions(env, path, flags).IsNothing()) return;
2324+
2325+
FSReqWrapSync req_open("open", *path);
2326+
2327+
FS_SYNC_TRACE_BEGIN(open);
2328+
file =
2329+
SyncCallAndThrowOnError(env, &req_open, uv_fs_open, *path, flags, mode);
2330+
FS_SYNC_TRACE_END(open);
2331+
2332+
if (is_uv_error(file)) {
2333+
return;
2334+
}
2335+
}
2336+
2337+
int bytesWritten = 0;
2338+
uint32_t offset = 0;
2339+
2340+
const size_t length = value.length();
2341+
uv_buf_t uvbuf = uv_buf_init(value.out(), length);
2342+
2343+
FS_SYNC_TRACE_BEGIN(write);
2344+
while (offset < length) {
2345+
FSReqWrapSync req_write("write");
2346+
bytesWritten = SyncCallAndThrowOnError(
2347+
env, &req_write, uv_fs_write, file, &uvbuf, 1, -1);
2348+
2349+
// Write errored out
2350+
if (bytesWritten < 0) {
2351+
break;
2352+
}
2353+
2354+
offset += bytesWritten;
2355+
DCHECK_LE(offset, length);
2356+
uvbuf.base += bytesWritten;
2357+
uvbuf.len -= bytesWritten;
2358+
}
2359+
FS_SYNC_TRACE_END(write);
2360+
2361+
if (!is_fd) {
2362+
FSReqWrapSync req_close("close");
2363+
2364+
FS_SYNC_TRACE_BEGIN(close);
2365+
int result = SyncCallAndThrowOnError(env, &req_close, uv_fs_close, file);
2366+
FS_SYNC_TRACE_END(close);
2367+
2368+
if (is_uv_error(result)) {
2369+
return;
2370+
}
2371+
}
2372+
}
22952373

22962374
/*
22972375
* Wrapper for read(2).
@@ -3134,6 +3212,7 @@ static void CreatePerIsolateProperties(IsolateData* isolate_data,
31343212
SetMethod(isolate, target, "writeBuffer", WriteBuffer);
31353213
SetMethod(isolate, target, "writeBuffers", WriteBuffers);
31363214
SetMethod(isolate, target, "writeString", WriteString);
3215+
SetMethod(isolate, target, "writeFileUtf8", WriteFileUtf8);
31373216
SetMethod(isolate, target, "realpath", RealPath);
31383217
SetMethod(isolate, target, "copyFile", CopyFile);
31393218

@@ -3254,6 +3333,7 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
32543333
registry->Register(WriteBuffer);
32553334
registry->Register(WriteBuffers);
32563335
registry->Register(WriteString);
3336+
registry->Register(WriteFileUtf8);
32573337
registry->Register(RealPath);
32583338
registry->Register(CopyFile);
32593339

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)