Skip to content

Commit 938471e

Browse files
anonrigruyadorno
authored andcommitted
fs: improve error performance of sync methods
PR-URL: #49593 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Robert Nagy <ronagy@icloud.com>
1 parent 1424404 commit 938471e

10 files changed

+529
-198
lines changed

benchmark/fs/bench-accessSync.js

+42
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
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 tmpfile = tmpdir.resolve(`.existing-file-${process.pid}`);
9+
fs.writeFileSync(tmpfile, 'this-is-for-a-benchmark', 'utf8');
10+
11+
const bench = common.createBenchmark(main, {
12+
type: ['existing', 'non-existing', 'non-flat-existing'],
13+
n: [1e5],
14+
});
15+
16+
function main({ n, type }) {
17+
let path;
18+
19+
switch (type) {
20+
case 'existing':
21+
path = __filename;
22+
break;
23+
case 'non-flat-existing':
24+
path = tmpfile;
25+
break;
26+
case 'non-existing':
27+
path = tmpdir.resolve(`.non-existing-file-${process.pid}`);
28+
break;
29+
default:
30+
new Error('Invalid type');
31+
}
32+
33+
bench.start();
34+
for (let i = 0; i < n; i++) {
35+
try {
36+
fs.accessSync(path);
37+
} catch {
38+
// do nothing
39+
}
40+
}
41+
bench.end(n);
42+
}

benchmark/fs/bench-copyFileSync.js

+37
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
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: ['invalid', 'valid'],
10+
n: [1e4],
11+
});
12+
13+
function main({ n, type }) {
14+
tmpdir.refresh();
15+
const dest = tmpdir.resolve(`copy-file-bench-${process.pid}`);
16+
let path;
17+
18+
switch (type) {
19+
case 'invalid':
20+
path = tmpdir.resolve(`.existing-file-${process.pid}`);
21+
break;
22+
case 'valid':
23+
path = __filename;
24+
break;
25+
default:
26+
throw new Error('Invalid type');
27+
}
28+
bench.start();
29+
for (let i = 0; i < n; i++) {
30+
try {
31+
fs.copyFileSync(path, dest);
32+
} catch {
33+
// do nothing
34+
}
35+
}
36+
bench.end(n);
37+
}

benchmark/fs/bench-existsSync.js

+38
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
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 tmpfile = tmpdir.resolve(`.existing-file-${process.pid}`);
9+
fs.writeFileSync(tmpfile, 'this-is-for-a-benchmark', 'utf8');
10+
11+
const bench = common.createBenchmark(main, {
12+
type: ['existing', 'non-existing', 'non-flat-existing'],
13+
n: [1e6],
14+
});
15+
16+
function main({ n, type }) {
17+
let path;
18+
19+
switch (type) {
20+
case 'existing':
21+
path = __filename;
22+
break;
23+
case 'non-flat-existing':
24+
path = tmpfile;
25+
break;
26+
case 'non-existing':
27+
path = tmpdir.resolve(`.non-existing-file-${process.pid}`);
28+
break;
29+
default:
30+
new Error('Invalid type');
31+
}
32+
33+
bench.start();
34+
for (let i = 0; i < n; i++) {
35+
fs.existsSync(path);
36+
}
37+
bench.end(n);
38+
}

benchmark/fs/bench-openSync.js

+37
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
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: [1e5],
11+
});
12+
13+
function main({ n, type }) {
14+
let path;
15+
16+
switch (type) {
17+
case 'existing':
18+
path = __filename;
19+
break;
20+
case 'non-existing':
21+
path = tmpdir.resolve(`.non-existing-file-${process.pid}`);
22+
break;
23+
default:
24+
new Error('Invalid type');
25+
}
26+
27+
bench.start();
28+
for (let i = 0; i < n; i++) {
29+
try {
30+
const fd = fs.openSync(path, 'r', 0o666);
31+
fs.closeSync(fd);
32+
} catch {
33+
// do nothing
34+
}
35+
}
36+
bench.end(n);
37+
}

lib/fs.js

+11-68
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ const {
141141
validateObject,
142142
validateString,
143143
} = require('internal/validators');
144-
const { readFileSyncUtf8 } = require('internal/fs/read/utf8');
144+
const syncFs = require('internal/fs/sync');
145145

146146
let truncateWarn = true;
147147
let fs;
@@ -243,12 +243,7 @@ function access(path, mode, callback) {
243243
* @returns {void}
244244
*/
245245
function accessSync(path, mode) {
246-
path = getValidatedPath(path);
247-
mode = getValidMode(mode, 'access');
248-
249-
const ctx = { path };
250-
binding.access(pathModule.toNamespacedPath(path), mode, undefined, ctx);
251-
handleErrorFromBinding(ctx);
246+
syncFs.access(path, mode);
252247
}
253248

254249
/**
@@ -290,23 +285,7 @@ ObjectDefineProperty(exists, kCustomPromisifiedSymbol, {
290285
* @returns {boolean}
291286
*/
292287
function existsSync(path) {
293-
try {
294-
path = getValidatedPath(path);
295-
} catch {
296-
return false;
297-
}
298-
const ctx = { path };
299-
const nPath = pathModule.toNamespacedPath(path);
300-
binding.access(nPath, F_OK, undefined, ctx);
301-
302-
// In case of an invalid symlink, `binding.access()` on win32
303-
// will **not** return an error and is therefore not enough.
304-
// Double check with `binding.stat()`.
305-
if (isWindows && ctx.errno === undefined) {
306-
binding.stat(nPath, false, undefined, ctx);
307-
}
308-
309-
return ctx.errno === undefined;
288+
return syncFs.exists(path);
310289
}
311290

312291
function readFileAfterOpen(err, fd) {
@@ -462,8 +441,7 @@ function readFileSync(path, options) {
462441

463442
// TODO(@anonrig): Do not handle file descriptor ownership for now.
464443
if (!isUserFd && (options.encoding === 'utf8' || options.encoding === 'utf-8')) {
465-
path = getValidatedPath(path);
466-
return readFileSyncUtf8(pathModule.toNamespacedPath(path), stringToFlags(options.flag));
444+
return syncFs.readFileUtf8(path, options.flag);
467445
}
468446

469447
const fd = isUserFd ? path : fs.openSync(path, options.flag, 0o666);
@@ -540,11 +518,7 @@ function close(fd, callback = defaultCloseCallback) {
540518
* @returns {void}
541519
*/
542520
function closeSync(fd) {
543-
fd = getValidatedFd(fd);
544-
545-
const ctx = {};
546-
binding.close(fd, undefined, ctx);
547-
handleErrorFromBinding(ctx);
521+
return syncFs.close(fd);
548522
}
549523

550524
/**
@@ -590,16 +564,7 @@ function open(path, flags, mode, callback) {
590564
* @returns {number}
591565
*/
592566
function openSync(path, flags, mode) {
593-
path = getValidatedPath(path);
594-
const flagsNumber = stringToFlags(flags);
595-
mode = parseFileMode(mode, 'mode', 0o666);
596-
597-
const ctx = { path };
598-
const result = binding.open(pathModule.toNamespacedPath(path),
599-
flagsNumber, mode,
600-
undefined, ctx);
601-
handleErrorFromBinding(ctx);
602-
return result;
567+
return syncFs.open(path, flags, mode);
603568
}
604569

605570
/**
@@ -1702,25 +1667,12 @@ function lstatSync(path, options = { bigint: false, throwIfNoEntry: true }) {
17021667
* }} [options]
17031668
* @returns {Stats}
17041669
*/
1705-
function statSync(path, options = { bigint: false, throwIfNoEntry: true }) {
1706-
path = getValidatedPath(path);
1707-
const ctx = { path };
1708-
const stats = binding.stat(pathModule.toNamespacedPath(path),
1709-
options.bigint, undefined, ctx);
1710-
if (options.throwIfNoEntry === false && hasNoEntryError(ctx)) {
1711-
return undefined;
1712-
}
1713-
handleErrorFromBinding(ctx);
1714-
return getStatsFromBinding(stats);
1670+
function statSync(path, options) {
1671+
return syncFs.stat(path, options);
17151672
}
17161673

1717-
function statfsSync(path, options = { bigint: false }) {
1718-
path = getValidatedPath(path);
1719-
const ctx = { path };
1720-
const stats = binding.statfs(pathModule.toNamespacedPath(path),
1721-
options.bigint, undefined, ctx);
1722-
handleErrorFromBinding(ctx);
1723-
return getStatFsFromBinding(stats);
1674+
function statfsSync(path, options) {
1675+
return syncFs.statfs(path, options);
17241676
}
17251677

17261678
/**
@@ -2999,16 +2951,7 @@ function copyFile(src, dest, mode, callback) {
29992951
* @returns {void}
30002952
*/
30012953
function copyFileSync(src, dest, mode) {
3002-
src = getValidatedPath(src, 'src');
3003-
dest = getValidatedPath(dest, 'dest');
3004-
3005-
const ctx = { path: src, dest }; // non-prefixed
3006-
3007-
src = pathModule._makeLong(src);
3008-
dest = pathModule._makeLong(dest);
3009-
mode = getValidMode(mode, 'copyFile');
3010-
binding.copyFile(src, dest, mode, undefined, ctx);
3011-
handleErrorFromBinding(ctx);
2954+
syncFs.copyFile(src, dest, mode);
30122955
}
30132956

30142957
/**

lib/internal/fs/read/utf8.js

-25
This file was deleted.

0 commit comments

Comments
 (0)