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 <[email protected]> Reviewed-By: Santiago Gimeno <[email protected]> Reviewed-By: Stephen Belanger <[email protected]> Reviewed-By: James M Snell <[email protected]>
1 parent 4cf155c commit aa3209b

File tree

5 files changed

+156
-0
lines changed

5 files changed

+156
-0
lines changed
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
'use strict';
2+
3+
constcommon=require('../common.js');
4+
constfs=require('fs');
5+
consttmpdir=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+
constbench=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+
functionmain({ n, func, encoding, length, useFd, useBuffer }){
24+
tmpdir.refresh();
25+
constenc=encoding==='undefined' ? undefined : encoding;
26+
constpath=tmpdir.resolve(`.writefilesync-file-${Date.now()}`);
27+
28+
useFd=useFd==='true';
29+
constfile=useFd ? fs.openSync(path,'w') : path;
30+
31+
letdata='a'.repeat(length);
32+
if(useBuffer==='true')data=Buffer.from(data,encoding);
33+
34+
constfn=fs[func+'Sync'];
35+
36+
bench.start();
37+
for(leti=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‎

Lines changed: 13 additions & 0 deletions
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(typeofdata==='string'&&(options.encoding==='utf8'||options.encoding==='utf-8')){
2346+
if(!isInt32(path)){
2347+
path=pathModule.toNamespacedPath(getValidatedPath(path));
2348+
}
2349+
2350+
returnbinding.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‎

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

2330+
staticvoidWriteFileUtf8(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+
constint flags = args[2].As<Int32>()->Value();
2344+
2345+
CHECK(args[3]->IsInt32());
2346+
constint 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+
constsize_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‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,17 +41,34 @@ fs.writeSync = function(){
4141
thrownewError('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+
thrownewError('BAM');
49+
};
50+
4451
internalBinding('fs').fstat=function(){
4552
thrownewError('EBADF: bad file descriptor, fstat');
4653
};
4754

4855
letclose_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
functionensureThrows(cb,message){
5774
letgot_exception=false;

‎typings/internalBinding/fs.d.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,9 @@ declare namespace InternalFSBinding{
231231
functionwriteString(fd: number,value: string,pos: unknown,encoding: unknown,usePromises: typeofkUsePromises): Promise<number>;
232232

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

236239
exportinterfaceFsBinding{

0 commit comments

Comments
(0)