Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 6bc7fa7

Browse filesBrowse files
CanadaHonkanonrig
CanadaHonk
authored and
anonrig
committed
fs: improve error perf of sync chmod+fchmod
PR-URL: #49859 Refs: nodejs/performance#106 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Darshan Sen <raisinten@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
1 parent 6bd77db commit 6bc7fa7
Copy full SHA for 6bc7fa7

File tree

Expand file treeCollapse file tree

5 files changed

+121
-24
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

5 files changed

+121
-24
lines changed
Open diff view settings
Collapse file

‎benchmark/fs/bench-chmodSync.js‎

Copy file name to clipboard
+41Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
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

‎benchmark/fs/bench-fchmodSync.js‎

Copy file name to clipboard
+60Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
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

‎lib/fs.js‎

Copy file name to clipboardExpand all lines: lib/fs.js
+8-8Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1903,11 +1903,10 @@ function fchmod(fd, mode, callback) {
19031903
* @returns {void}
19041904
*/
19051905
function fchmodSync(fd, mode) {
1906-
fd = getValidatedFd(fd);
1907-
mode = parseFileMode(mode, 'mode');
1908-
const ctx = {};
1909-
binding.fchmod(fd, mode, undefined, ctx);
1910-
handleErrorFromBinding(ctx);
1906+
binding.fchmod(
1907+
getValidatedFd(fd),
1908+
parseFileMode(mode, 'mode'),
1909+
);
19111910
}
19121911

19131912
/**
@@ -1982,9 +1981,10 @@ function chmodSync(path, mode) {
19821981
path = getValidatedPath(path);
19831982
mode = parseFileMode(mode, 'mode');
19841983

1985-
const ctx = { path };
1986-
binding.chmod(pathModule.toNamespacedPath(path), mode, undefined, ctx);
1987-
handleErrorFromBinding(ctx);
1984+
binding.chmod(
1985+
pathModule.toNamespacedPath(path),
1986+
mode,
1987+
);
19881988
}
19891989

19901990
/**
Collapse file

‎src/node_file.cc‎

Copy file name to clipboardExpand all lines: src/node_file.cc
+10-14Lines changed: 10 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2517,18 +2517,16 @@ static void Chmod(const FunctionCallbackInfo<Value>& args) {
25172517
CHECK(args[1]->IsInt32());
25182518
int mode = args[1].As<Int32>()->Value();
25192519

2520-
FSReqBase* req_wrap_async = GetReqWrap(args, 2);
2521-
if (req_wrap_async != nullptr) { // chmod(path, mode, req)
2520+
if (argc > 2) { // chmod(path, mode, req)
2521+
FSReqBase* req_wrap_async = GetReqWrap(args, 2);
25222522
FS_ASYNC_TRACE_BEGIN1(
25232523
UV_FS_CHMOD, req_wrap_async, "path", TRACE_STR_COPY(*path))
25242524
AsyncCall(env, req_wrap_async, args, "chmod", UTF8, AfterNoArgs,
25252525
uv_fs_chmod, *path, mode);
2526-
} else { // chmod(path, mode, undefined, ctx)
2527-
CHECK_EQ(argc, 4);
2528-
FSReqWrapSync req_wrap_sync;
2526+
} else { // chmod(path, mode)
2527+
FSReqWrapSync req_wrap_sync("chmod", *path);
25292528
FS_SYNC_TRACE_BEGIN(chmod);
2530-
SyncCall(env, args[3], &req_wrap_sync, "chmod",
2531-
uv_fs_chmod, *path, mode);
2529+
SyncCallAndThrowOnError(env, &req_wrap_sync, uv_fs_chmod, *path, mode);
25322530
FS_SYNC_TRACE_END(chmod);
25332531
}
25342532
}
@@ -2549,17 +2547,15 @@ static void FChmod(const FunctionCallbackInfo<Value>& args) {
25492547
CHECK(args[1]->IsInt32());
25502548
const int mode = args[1].As<Int32>()->Value();
25512549

2552-
FSReqBase* req_wrap_async = GetReqWrap(args, 2);
2553-
if (req_wrap_async != nullptr) { // fchmod(fd, mode, req)
2550+
if (argc > 2) { // fchmod(fd, mode, req)
2551+
FSReqBase* req_wrap_async = GetReqWrap(args, 2);
25542552
FS_ASYNC_TRACE_BEGIN0(UV_FS_FCHMOD, req_wrap_async)
25552553
AsyncCall(env, req_wrap_async, args, "fchmod", UTF8, AfterNoArgs,
25562554
uv_fs_fchmod, fd, mode);
2557-
} else { // fchmod(fd, mode, undefined, ctx)
2558-
CHECK_EQ(argc, 4);
2559-
FSReqWrapSync req_wrap_sync;
2555+
} else { // fchmod(fd, mode)
2556+
FSReqWrapSync req_wrap_sync("fchmod");
25602557
FS_SYNC_TRACE_BEGIN(fchmod);
2561-
SyncCall(env, args[3], &req_wrap_sync, "fchmod",
2562-
uv_fs_fchmod, fd, mode);
2558+
SyncCallAndThrowOnError(env, &req_wrap_sync, uv_fs_fchmod, fd, mode);
25632559
FS_SYNC_TRACE_END(fchmod);
25642560
}
25652561
}
Collapse file

‎typings/internalBinding/fs.d.ts‎

Copy file name to clipboardExpand all lines: typings/internalBinding/fs.d.ts
+2-2Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ declare namespace InternalFSBinding {
6161
function access(path: StringOrBuffer, mode: number, usePromises: typeof kUsePromises): Promise<void>;
6262

6363
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;
6565
function chmod(path: string, mode: number, usePromises: typeof kUsePromises): Promise<void>;
6666

6767
function chown(path: string, uid: number, gid: number, req: FSReqCallback): void;
@@ -76,7 +76,7 @@ declare namespace InternalFSBinding {
7676
function copyFile(src: StringOrBuffer, dest: StringOrBuffer, mode: number, usePromises: typeof kUsePromises): Promise<void>;
7777

7878
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;
8080
function fchmod(fd: number, mode: number, usePromises: typeof kUsePromises): Promise<void>;
8181

8282
function fchown(fd: number, uid: number, gid: number, req: FSReqCallback): void;

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.