GeezerVison is showing github.com — fs: improve error perf of sync `chmod`+`fchmod` · nodejs/node@6ddea07
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 +41Lines changed: 41 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 bench = common.createBenchmark(main, { | | 9 | + type: ['existing', 'non-existing'], | | 10 | + n: [1e3], | | 11 | +}); | | 12 | + | | 13 | +function main({ n, type }) { | | 14 | + switch (type) { | | 15 | + case 'existing': { | | 16 | + for (let i = 0; i < n; i++) { | | 17 | + fs.writeFileSync(tmpdir.resolve(`chmodsync-bench-file-${i}`), 'bench'); | | 18 | + } | | 19 | + | | 20 | + bench.start(); | | 21 | + for (let i = 0; i < n; i++) { | | 22 | + fs.chmodSync(tmpdir.resolve(`chmodsync-bench-file-${i}`), 0o666); | | 23 | + } | | 24 | + bench.end(n); | | 25 | + break; | | 26 | + } | | 27 | + case 'non-existing': | | 28 | + bench.start(); | | 29 | + for (let i = 0; i < n; i++) { | | 30 | + try { | | 31 | + fs.chmodSync(tmpdir.resolve(`chmod-non-existing-file-${i}`), 0o666); | | 32 | + } catch { | | 33 | + // do nothing | | 34 | + } | | 35 | + } | | 36 | + bench.end(n); | | 37 | + break; | | 38 | + default: | | 39 | + new Error('Invalid type'); | | 40 | + } | | 41 | +} |
Collapse file Copy file name to clipboard +60Lines changed: 60 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 bench = common.createBenchmark(main, { | | 9 | + type: ['existing', 'non-existing'], | | 10 | + n: [1e3], | | 11 | +}); | | 12 | + | | 13 | +function main({ n, type }) { | | 14 | + let files; | | 15 | + | | 16 | + switch (type) { | | 17 | + case 'existing': | | 18 | + files = []; | | 19 | + | | 20 | + // Populate tmpdir with mock files | | 21 | + for (let i = 0; i < n; i++) { | | 22 | + const path = tmpdir.resolve(`fchmodsync-bench-file-${i}`); | | 23 | + fs.writeFileSync(path, 'bench'); | | 24 | + files.push(path); | | 25 | + } | | 26 | + break; | | 27 | + case 'non-existing': | | 28 | + files = new Array(n).fill(tmpdir.resolve(`.non-existing-file-${Date.now()}`)); | | 29 | + break; | | 30 | + default: | | 31 | + new Error('Invalid type'); | | 32 | + } | | 33 | + | | 34 | + const fds = files.map((x) => { | | 35 | + // Try to open, if not return likely invalid fd (1 << 30) | | 36 | + try { | | 37 | + return fs.openSync(x, 'r'); | | 38 | + } catch { | | 39 | + return 1 << 30; | | 40 | + } | | 41 | + }); | | 42 | + | | 43 | + bench.start(); | | 44 | + for (let i = 0; i < n; i++) { | | 45 | + try { | | 46 | + fs.fchmodSync(fds[i], 0o666); | | 47 | + } catch { | | 48 | + // do nothing | | 49 | + } | | 50 | + } | | 51 | + bench.end(n); | | 52 | + | | 53 | + for (const x of fds) { | | 54 | + try { | | 55 | + fs.closeSync(x); | | 56 | + } catch { | | 57 | + // do nothing | | 58 | + } | | 59 | + } | | 60 | +} |
Collapse file Copy file name to clipboardExpand all lines: lib/fs.js +8-8Lines changed: 8 additions & 8 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -1901,11 +1901,10 @@ function fchmod(fd, mode, callback) { | 1901 | 1901 | * @returns {void} | 1902 | 1902 | */ | 1903 | 1903 | function fchmodSync(fd, mode) { | 1904 | | - fd = getValidatedFd(fd); | 1905 | | - mode = parseFileMode(mode, 'mode'); | 1906 | | - const ctx = {}; | 1907 | | - binding.fchmod(fd, mode, undefined, ctx); | 1908 | | - handleErrorFromBinding(ctx); | | 1904 | + binding.fchmod( | | 1905 | + getValidatedFd(fd), | | 1906 | + parseFileMode(mode, 'mode'), | | 1907 | + ); | 1909 | 1908 | } | 1910 | 1909 |
| 1911 | 1910 | /** | @@ -1980,9 +1979,10 @@ function chmodSync(path, mode) { | 1980 | 1979 | path = getValidatedPath(path); | 1981 | 1980 | mode = parseFileMode(mode, 'mode'); | 1982 | 1981 |
| 1983 | | - const ctx = { path }; | 1984 | | - binding.chmod(pathModule.toNamespacedPath(path), mode, undefined, ctx); | 1985 | | - handleErrorFromBinding(ctx); | | 1982 | + binding.chmod( | | 1983 | + pathModule.toNamespacedPath(path), | | 1984 | + mode, | | 1985 | + ); | 1986 | 1986 | } | 1987 | 1987 |
| 1988 | 1988 | /** | |
Collapse file Copy file name to clipboardExpand all lines: src/node_file.cc +10-14Lines changed: 10 additions & 14 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -2552,18 +2552,16 @@ static void Chmod(const FunctionCallbackInfo<Value>& args) { | 2552 | 2552 | CHECK(args[1]->IsInt32()); | 2553 | 2553 | int mode = args[1].As<Int32>()->Value(); | 2554 | 2554 |
| 2555 | | - FSReqBase* req_wrap_async = GetReqWrap(args, 2); | 2556 | | - if (req_wrap_async != nullptr) { // chmod(path, mode, req) | | 2555 | + if (argc > 2) { // chmod(path, mode, req) | | 2556 | + FSReqBase* req_wrap_async = GetReqWrap(args, 2); | 2557 | 2557 | FS_ASYNC_TRACE_BEGIN1( | 2558 | 2558 | UV_FS_CHMOD, req_wrap_async, "path", TRACE_STR_COPY(*path)) | 2559 | 2559 | AsyncCall(env, req_wrap_async, args, "chmod", UTF8, AfterNoArgs, | 2560 | 2560 | uv_fs_chmod, *path, mode); | 2561 | | - } else { // chmod(path, mode, undefined, ctx) | 2562 | | - CHECK_EQ(argc, 4); | 2563 | | - FSReqWrapSync req_wrap_sync; | | 2561 | + } else { // chmod(path, mode) | | 2562 | + FSReqWrapSync req_wrap_sync("chmod", *path); | 2564 | 2563 | FS_SYNC_TRACE_BEGIN(chmod); | 2565 | | - SyncCall(env, args[3], &req_wrap_sync, "chmod", | 2566 | | - uv_fs_chmod, *path, mode); | | 2564 | + SyncCallAndThrowOnError(env, &req_wrap_sync, uv_fs_chmod, *path, mode); | 2567 | 2565 | FS_SYNC_TRACE_END(chmod); | 2568 | 2566 | } | 2569 | 2567 | } | @@ -2584,17 +2582,15 @@ static void FChmod(const FunctionCallbackInfo<Value>& args) { | 2584 | 2582 | CHECK(args[1]->IsInt32()); | 2585 | 2583 | const int mode = args[1].As<Int32>()->Value(); | 2586 | 2584 |
| 2587 | | - FSReqBase* req_wrap_async = GetReqWrap(args, 2); | 2588 | | - if (req_wrap_async != nullptr) { // fchmod(fd, mode, req) | | 2585 | + if (argc > 2) { // fchmod(fd, mode, req) | | 2586 | + FSReqBase* req_wrap_async = GetReqWrap(args, 2); | 2589 | 2587 | FS_ASYNC_TRACE_BEGIN0(UV_FS_FCHMOD, req_wrap_async) | 2590 | 2588 | AsyncCall(env, req_wrap_async, args, "fchmod", UTF8, AfterNoArgs, | 2591 | 2589 | uv_fs_fchmod, fd, mode); | 2592 | | - } else { // fchmod(fd, mode, undefined, ctx) | 2593 | | - CHECK_EQ(argc, 4); | 2594 | | - FSReqWrapSync req_wrap_sync; | | 2590 | + } else { // fchmod(fd, mode) | | 2591 | + FSReqWrapSync req_wrap_sync("fchmod"); | 2595 | 2592 | FS_SYNC_TRACE_BEGIN(fchmod); | 2596 | | - SyncCall(env, args[3], &req_wrap_sync, "fchmod", | 2597 | | - uv_fs_fchmod, fd, mode); | | 2593 | + SyncCallAndThrowOnError(env, &req_wrap_sync, uv_fs_fchmod, fd, mode); | 2598 | 2594 | FS_SYNC_TRACE_END(fchmod); | 2599 | 2595 | } | 2600 | 2596 | } | |
Collapse file Copy file name to clipboardExpand all lines: typings/internalBinding/fs.d.ts +2-2Lines changed: 2 additions & 2 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -61,7 +61,7 @@ declare namespace InternalFSBinding { | 61 | 61 | function access(path: StringOrBuffer, mode: number, usePromises: typeof kUsePromises): Promise<void>; | 62 | 62 |
| 63 | 63 | function chmod(path: string, mode: number, req: FSReqCallback): void; | 64 | | - function chmod(path: string, mode: number, req: undefined, ctx: FSSyncContext): void; | | 64 | + function chmod(path: string, mode: number): void; | 65 | 65 | function chmod(path: string, mode: number, usePromises: typeof kUsePromises): Promise<void>; | 66 | 66 |
| 67 | 67 | function chown(path: string, uid: number, gid: number, req: FSReqCallback): void; | @@ -76,7 +76,7 @@ declare namespace InternalFSBinding { | 76 | 76 | function copyFile(src: StringOrBuffer, dest: StringOrBuffer, mode: number, usePromises: typeof kUsePromises): Promise<void>; | 77 | 77 |
| 78 | 78 | function fchmod(fd: number, mode: number, req: FSReqCallback): void; | 79 | | - function fchmod(fd: number, mode: number, req: undefined, ctx: FSSyncContext): void; | | 79 | + function fchmod(fd: number, mode: number): void; | 80 | 80 | function fchmod(fd: number, mode: number, usePromises: typeof kUsePromises): Promise<void>; | 81 | 81 |
| 82 | 82 | function fchown(fd: number, uid: number, gid: number, req: FSReqCallback): void; | |
|