GeezerVison is showing github.com — fs: improve error performance for `readSync` · nodejs/node@e49ebf8
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 |
|---|
@@ -749,11 +749,7 @@ function readSync(fd, buffer, offsetOrOptions, length, position) { | 749 | 749 |
| 750 | 750 | validatePosition(position, 'position'); | 751 | 751 |
| 752 | | - const ctx = {}; | 753 | | - const result = binding.read(fd, buffer, offset, length, position, | 754 | | - undefined, ctx); | 755 | | - handleErrorFromBinding(ctx); | 756 | | - return result; | | 752 | + return binding.read(fd, buffer, offset, length, position); | 757 | 753 | } | 758 | 754 |
| 759 | 755 | /** | |
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 |
|---|
@@ -2376,18 +2376,23 @@ static void Read(const FunctionCallbackInfo<Value>& args) { | 2376 | 2376 | char* buf = buffer_data + off; | 2377 | 2377 | uv_buf_t uvbuf = uv_buf_init(buf, len); | 2378 | 2378 |
| 2379 | | - FSReqBase* req_wrap_async = GetReqWrap(args, 5); | 2380 | | - if (req_wrap_async != nullptr) { // read(fd, buffer, offset, len, pos, req) | | 2379 | + if (argc > 5) { // read(fd, buffer, offset, len, pos, req) | | 2380 | + FSReqBase* req_wrap_async = GetReqWrap(args, 5); | | 2381 | + CHECK_NOT_NULL(req_wrap_async); | 2381 | 2382 | FS_ASYNC_TRACE_BEGIN0(UV_FS_READ, req_wrap_async) | 2382 | 2383 | AsyncCall(env, req_wrap_async, args, "read", UTF8, AfterInteger, | 2383 | 2384 | uv_fs_read, fd, &uvbuf, 1, pos); | 2384 | | - } else { // read(fd, buffer, offset, len, pos, undefined, ctx) | 2385 | | - CHECK_EQ(argc, 7); | 2386 | | - FSReqWrapSync req_wrap_sync; | | 2385 | + } else { // read(fd, buffer, offset, len, pos) | | 2386 | + FSReqWrapSync req_wrap_sync("read"); | 2387 | 2387 | FS_SYNC_TRACE_BEGIN(read); | 2388 | | - const int bytesRead = SyncCall(env, args[6], &req_wrap_sync, "read", | 2389 | | - uv_fs_read, fd, &uvbuf, 1, pos); | | 2388 | + const int bytesRead = SyncCallAndThrowOnError( | | 2389 | + env, &req_wrap_sync, uv_fs_read, fd, &uvbuf, 1, pos); | 2390 | 2390 | FS_SYNC_TRACE_END(read, "bytesRead", bytesRead); | | 2391 | + | | 2392 | + if (is_uv_error(bytesRead)) { | | 2393 | + return; | | 2394 | + } | | 2395 | + | 2391 | 2396 | args.GetReturnValue().Set(bytesRead); | 2392 | 2397 | } | 2393 | 2398 | } | |
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; | |
|