GeezerVison is showing github.com — fs: add c++ fast path for writeFileSync utf8 · nodejs/node@aa3209b
File treeExpand file treeCollapse file tree Open diff view settings Expand file treeCollapse file tree Open diff view settings Collapse file Copy file name to clipboard +43Lines changed: 43 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
| | 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 | +} |
Collapse file Copy file name to clipboardExpand all lines: lib/fs.js +13Lines changed: 13 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -2341,6 +2341,19 @@ function writeFileSync(path, data, options) { | 2341 | 2341 |
| 2342 | 2342 | validateBoolean(flush, 'options.flush'); | 2343 | 2343 |
| | 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 | + | 2344 | 2357 | if (!isArrayBufferView(data)) { | 2345 | 2358 | validateStringAfterArrayBufferView(data, 'data'); | 2346 | 2359 | data = Buffer.from(data, options.encoding || 'utf8'); | |
Collapse file Copy file name to clipboardExpand all lines: src/node_file.cc +80Lines changed: 80 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -2327,6 +2327,84 @@ static void WriteString(const FunctionCallbackInfo<Value>& args) { | 2327 | 2327 | } | 2328 | 2328 | } | 2329 | 2329 |
| | 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 | +} | 2330 | 2408 |
| 2331 | 2409 | /* | 2332 | 2410 | * Wrapper for read(2). | @@ -3169,6 +3247,7 @@ static void CreatePerIsolateProperties(IsolateData* isolate_data, | 3169 | 3247 | SetMethod(isolate, target, "writeBuffer", WriteBuffer); | 3170 | 3248 | SetMethod(isolate, target, "writeBuffers", WriteBuffers); | 3171 | 3249 | SetMethod(isolate, target, "writeString", WriteString); | | 3250 | + SetMethod(isolate, target, "writeFileUtf8", WriteFileUtf8); | 3172 | 3251 | SetMethod(isolate, target, "realpath", RealPath); | 3173 | 3252 | SetMethod(isolate, target, "copyFile", CopyFile); | 3174 | 3253 |
| @@ -3289,6 +3368,7 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) { | 3289 | 3368 | registry->Register(WriteBuffer); | 3290 | 3369 | registry->Register(WriteBuffers); | 3291 | 3370 | registry->Register(WriteString); | | 3371 | + registry->Register(WriteFileUtf8); | 3292 | 3372 | registry->Register(RealPath); | 3293 | 3373 | registry->Register(CopyFile); | 3294 | 3374 |
| |
Collapse file Copy file name to clipboardExpand all lines: test/parallel/test-fs-sync-fd-leak.js +17Lines changed: 17 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -41,17 +41,34 @@ fs.writeSync = function() { | 41 | 41 | throw new Error('BAM'); | 42 | 42 | }; | 43 | 43 |
| | 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 | + | 44 | 51 | internalBinding('fs').fstat = function() { | 45 | 52 | throw new Error('EBADF: bad file descriptor, fstat'); | 46 | 53 | }; | 47 | 54 |
| 48 | 55 | let close_called = 0; | 49 | 56 | ensureThrows(function() { | | 57 | + // Fast path: writeFileSync utf8 | 50 | 58 | fs.writeFileSync('dummy', 'xxx'); | 51 | 59 | }, 'BAM'); | 52 | 60 | ensureThrows(function() { | | 61 | + // Non-fast path | | 62 | + fs.writeFileSync('dummy', 'xxx', { encoding: 'base64' }); | | 63 | +}, 'BAM'); | | 64 | +ensureThrows(function() { | | 65 | + // Fast path: writeFileSync utf8 | 53 | 66 | fs.appendFileSync('dummy', 'xxx'); | 54 | 67 | }, 'BAM'); | | 68 | +ensureThrows(function() { | | 69 | + // Non-fast path | | 70 | + fs.appendFileSync('dummy', 'xxx', { encoding: 'base64' }); | | 71 | +}, 'BAM'); | 55 | 72 |
| 56 | 73 | function ensureThrows(cb, message) { | 57 | 74 | let got_exception = false; | |
Collapse file Copy file name to clipboardExpand all lines: typings/internalBinding/fs.d.ts +3Lines changed: 3 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -231,6 +231,9 @@ declare namespace InternalFSBinding { | 231 | 231 | function writeString(fd: number, value: string, pos: unknown, encoding: unknown, usePromises: typeof kUsePromises): Promise<number>; | 232 | 232 |
| 233 | 233 | 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; | 234 | 237 | } | 235 | 238 |
| 236 | 239 | export interface FsBinding { | |
|