GeezerVison is showing github.com — fs: improve error performance for `readSync` · nodejs/node@0cc176e
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 +57Lines changed: 57 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
| | 1 | +'use strict'; | | 2 | + | | 3 | +const common = require('../common'); | | 4 | +const fs = require('fs'); | | 5 | +const tmpdir = require('../../test/common/tmpdir'); | | 6 | +tmpdir.refresh(); | | 7 | + | | 8 | +const bufferSize = 1024; | | 9 | +const sectorSize = 512; | | 10 | + | | 11 | +const bench = common.createBenchmark(main, { | | 12 | + type: ['existing', 'non-existing'], | | 13 | + n: [1e4], | | 14 | +}); | | 15 | + | | 16 | +function main({ n, type }) { | | 17 | + let fd; | | 18 | + | | 19 | + const tmpfile = { name: tmpdir.resolve(`.existing-file-${process.pid}`), | | 20 | + len: bufferSize * n }; | | 21 | + | | 22 | + | | 23 | + tmpfile.contents = Buffer.allocUnsafe(tmpfile.len); | | 24 | + | | 25 | + for (let offset = 0; offset < tmpfile.len; offset += sectorSize) { | | 26 | + const fillByte = 256 * Math.random(); | | 27 | + const nBytesToFill = Math.min(sectorSize, tmpfile.len - offset); | | 28 | + tmpfile.contents.fill(fillByte, offset, offset + nBytesToFill); | | 29 | + } | | 30 | + | | 31 | + fs.writeFileSync(tmpfile.name, tmpfile.contents); | | 32 | + | | 33 | + switch (type) { | | 34 | + case 'existing': | | 35 | + fd = fs.openSync(tmpfile.name, 'r', 0o666); | | 36 | + break; | | 37 | + case 'non-existing': | | 38 | + fd = 1 << 30; | | 39 | + break; | | 40 | + default: | | 41 | + new Error('Invalid type'); | | 42 | + } | | 43 | + | | 44 | + const buffer = Buffer.alloc(bufferSize); | | 45 | + | | 46 | + bench.start(); | | 47 | + for (let i = 0; i < n; i++) { | | 48 | + try { | | 49 | + fs.readSync(fd, buffer); | | 50 | + } catch { | | 51 | + // Continue regardless of error. | | 52 | + } | | 53 | + } | | 54 | + bench.end(n); | | 55 | + | | 56 | + if (type === 'existing') fs.closeSync(fd); | | 57 | +} |
Collapse file Copy file name to clipboardExpand all lines: lib/fs.js +1-5Lines changed: 1 addition & 5 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -751,11 +751,7 @@ function readSync(fd, buffer, offsetOrOptions, length, position) { | 751 | 751 | validatePosition(position, 'position', length); | 752 | 752 | } | 753 | 753 |
| 754 | | - const ctx = {}; | 755 | | - const result = binding.read(fd, buffer, offset, length, position, | 756 | | - undefined, ctx); | 757 | | - handleErrorFromBinding(ctx); | 758 | | - return result; | | 754 | + return binding.read(fd, buffer, offset, length, position); | 759 | 755 | } | 760 | 756 |
| 761 | 757 | /** | |
Collapse file Copy file name to clipboardExpand all lines: src/node_file.cc +12-7Lines changed: 12 additions & 7 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -2341,18 +2341,23 @@ static void Read(const FunctionCallbackInfo<Value>& args) { | 2341 | 2341 | char* buf = buffer_data + off; | 2342 | 2342 | uv_buf_t uvbuf = uv_buf_init(buf, len); | 2343 | 2343 |
| 2344 | | - FSReqBase* req_wrap_async = GetReqWrap(args, 5); | 2345 | | - if (req_wrap_async != nullptr) { // read(fd, buffer, offset, len, pos, req) | | 2344 | + if (argc > 5) { // read(fd, buffer, offset, len, pos, req) | | 2345 | + FSReqBase* req_wrap_async = GetReqWrap(args, 5); | | 2346 | + CHECK_NOT_NULL(req_wrap_async); | 2346 | 2347 | FS_ASYNC_TRACE_BEGIN0(UV_FS_READ, req_wrap_async) | 2347 | 2348 | AsyncCall(env, req_wrap_async, args, "read", UTF8, AfterInteger, | 2348 | 2349 | uv_fs_read, fd, &uvbuf, 1, pos); | 2349 | | - } else { // read(fd, buffer, offset, len, pos, undefined, ctx) | 2350 | | - CHECK_EQ(argc, 7); | 2351 | | - FSReqWrapSync req_wrap_sync; | | 2350 | + } else { // read(fd, buffer, offset, len, pos) | | 2351 | + FSReqWrapSync req_wrap_sync("read"); | 2352 | 2352 | FS_SYNC_TRACE_BEGIN(read); | 2353 | | - const int bytesRead = SyncCall(env, args[6], &req_wrap_sync, "read", | 2354 | | - uv_fs_read, fd, &uvbuf, 1, pos); | | 2353 | + const int bytesRead = SyncCallAndThrowOnError( | | 2354 | + env, &req_wrap_sync, uv_fs_read, fd, &uvbuf, 1, pos); | 2355 | 2355 | FS_SYNC_TRACE_END(read, "bytesRead", bytesRead); | | 2356 | + | | 2357 | + if (is_uv_error(bytesRead)) { | | 2358 | + return; | | 2359 | + } | | 2360 | + | 2356 | 2361 | args.GetReturnValue().Set(bytesRead); | 2357 | 2362 | } | 2358 | 2363 | } | |
Collapse file Copy file name to clipboardExpand all lines: typings/internalBinding/fs.d.ts +1-1Lines changed: 1 addition & 1 deletion | Original file line number | Diff line number | Diff line change |
|---|
@@ -159,8 +159,8 @@ declare namespace InternalFSBinding { | 159 | 159 | function openFileHandle(path: StringOrBuffer, flags: number, mode: number, usePromises: typeof kUsePromises): Promise<FileHandle>; | 160 | 160 |
| 161 | 161 | function read(fd: number, buffer: ArrayBufferView, offset: number, length: number, position: number, req: FSReqCallback<number>): void; | 162 | | - function read(fd: number, buffer: ArrayBufferView, offset: number, length: number, position: number, req: undefined, ctx: FSSyncContext): number; | 163 | 162 | function read(fd: number, buffer: ArrayBufferView, offset: number, length: number, position: number, usePromises: typeof kUsePromises): Promise<number>; | | 163 | + function read(fd: number, buffer: ArrayBufferView, offset: number, length: number, position: number): number; | 164 | 164 |
| 165 | 165 | function readBuffers(fd: number, buffers: ArrayBufferView[], position: number, req: FSReqCallback<number>): void; | 166 | 166 | function readBuffers(fd: number, buffers: ArrayBufferView[], position: number, req: undefined, ctx: FSSyncContext): number; | |
|