Skip to content

Commit b32f918

Browse files
CanadaHonkanonrig
CanadaHonk
authored and
anonrig
committed
fs: improve error perf of sync lstat+fstat 
1 parent dc1c50b commit b32f918

File tree

5 files changed

+47
-54
lines changed

5 files changed

+47
-54
lines changed

‎benchmark/fs/bench-statSync-failure.js

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,21 +5,29 @@ const fs = require('fs');
55
const path = require('path');
66

77
const bench = common.createBenchmark(main, {
8-
n: [1e6],
9-
statSyncType: ['throw', 'noThrow'],
8+
n: [1e4],
9+
statSyncType: ['fstatSync', 'lstatSync', 'statSync'],
10+
throwType: [ 'throw', 'noThrow' ],
11+
}, {
12+
// Do not allow noThrow fstatSync
13+
combinationFilter: ({ statSyncType, throwType }) => !(statSyncType === 'fstatSync' && throwType === 'noThrow'),
1014
});
1115

1216

13-
function main({ n, statSyncType }) {
14-
const arg = path.join(__dirname, 'non.existent');
17+
function main({ n, statSyncType, throwType }) {
18+
const arg = (statSyncType === 'fstatSync' ?
19+
(1 << 30) :
20+
path.join(__dirname, 'non.existent'));
21+
22+
const fn = fs[statSyncType];
1523

1624
bench.start();
1725
for (let i = 0; i < n; i++) {
18-
if (statSyncType === 'noThrow') {
19-
fs.statSync(arg, { throwIfNoEntry: false });
26+
if (throwType === 'noThrow') {
27+
fn(arg, { throwIfNoEntry: false });
2028
} else {
2129
try {
22-
fs.statSync(arg);
30+
fn(arg);
2331
} catch {
2432
// Continue regardless of error.
2533
}

‎benchmark/fs/bench-statSync.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ const common = require('../common');
44
const fs = require('fs');
55

66
const bench = common.createBenchmark(main, {
7-
n: [1e6],
7+
n: [1e4],
88
statSyncType: ['fstatSync', 'lstatSync', 'statSync'],
99
});
1010

‎lib/fs.js

Lines changed: 11 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,6 @@ const {
7272
ERR_INVALID_ARG_VALUE,
7373
},
7474
AbortError,
75-
uvErrmapGet,
7675
uvException,
7776
} = require('internal/errors');
7877

@@ -1621,19 +1620,6 @@ function statfs(path, options = { bigint: false }, callback) {
16211620
binding.statfs(pathModule.toNamespacedPath(path), options.bigint, req);
16221621
}
16231622

1624-
function hasNoEntryError(ctx) {
1625-
if (ctx.errno) {
1626-
const uvErr = uvErrmapGet(ctx.errno);
1627-
return uvErr?.[0] === 'ENOENT';
1628-
}
1629-
1630-
if (ctx.error) {
1631-
return ctx.error.code === 'ENOENT';
1632-
}
1633-
1634-
return false;
1635-
}
1636-
16371623
/**
16381624
* Synchronously retrieves the `fs.Stats` for
16391625
* the file descriptor.
@@ -1645,9 +1631,10 @@ function hasNoEntryError(ctx) {
16451631
*/
16461632
function fstatSync(fd, options = { bigint: false }) {
16471633
fd = getValidatedFd(fd);
1648-
const ctx = { fd };
1649-
const stats = binding.fstat(fd, options.bigint, undefined, ctx);
1650-
handleErrorFromBinding(ctx);
1634+
const stats = binding.fstat(fd, options.bigint);
1635+
if (stats === undefined) {
1636+
return;
1637+
}
16511638
return getStatsFromBinding(stats);
16521639
}
16531640

@@ -1663,13 +1650,14 @@ function fstatSync(fd, options = { bigint: false }) {
16631650
*/
16641651
function lstatSync(path, options = { bigint: false, throwIfNoEntry: true }) {
16651652
path = getValidatedPath(path);
1666-
const ctx = { path };
1667-
const stats = binding.lstat(pathModule.toNamespacedPath(path),
1668-
options.bigint, undefined, ctx);
1669-
if (options.throwIfNoEntry === false && hasNoEntryError(ctx)) {
1670-
return undefined;
1653+
const stats = binding.lstat(
1654+
pathModule.toNamespacedPath(path),
1655+
options.bigint,
1656+
);
1657+
1658+
if (stats === undefined) {
1659+
return;
16711660
}
1672-
handleErrorFromBinding(ctx);
16731661
return getStatsFromBinding(stats);
16741662
}
16751663

‎src/node_file.cc

Lines changed: 14 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1186,27 +1186,25 @@ static void LStat(const FunctionCallbackInfo<Value>& args) {
11861186
Environment* env = realm->env();
11871187

11881188
const int argc = args.Length();
1189-
CHECK_GE(argc, 3);
1189+
CHECK_GE(argc, 2);
11901190

11911191
BufferValue path(realm->isolate(), args[0]);
11921192
CHECK_NOT_NULL(*path);
11931193

11941194
bool use_bigint = args[1]->IsTrue();
1195-
FSReqBase* req_wrap_async = GetReqWrap(args, 2, use_bigint);
1196-
if (req_wrap_async != nullptr) { // lstat(path, use_bigint, req)
1195+
if (argc > 2) { // lstat(path, use_bigint, req)
1196+
FSReqBase* req_wrap_async = GetReqWrap(args, 2, use_bigint);
11971197
FS_ASYNC_TRACE_BEGIN1(
11981198
UV_FS_LSTAT, req_wrap_async, "path", TRACE_STR_COPY(*path))
11991199
AsyncCall(env, req_wrap_async, args, "lstat", UTF8, AfterStat,
12001200
uv_fs_lstat, *path);
12011201
} else { // lstat(path, use_bigint, undefined, ctx)
1202-
CHECK_EQ(argc, 4);
1203-
FSReqWrapSync req_wrap_sync;
1202+
FSReqWrapSync req_wrap_sync("lstat", *path);
12041203
FS_SYNC_TRACE_BEGIN(lstat);
1205-
int err = SyncCall(env, args[3], &req_wrap_sync, "lstat", uv_fs_lstat,
1206-
*path);
1204+
int err = SyncCallAndThrowOnError(env, &req_wrap_sync, uv_fs_lstat, *path);
12071205
FS_SYNC_TRACE_END(lstat);
1208-
if (err != 0) {
1209-
return; // error info is in ctx
1206+
if (is_uv_error(err)) {
1207+
return;
12101208
}
12111209

12121210
Local<Value> arr = FillGlobalStatsArray(binding_data, use_bigint,
@@ -1227,19 +1225,18 @@ static void FStat(const FunctionCallbackInfo<Value>& args) {
12271225
int fd = args[0].As<Int32>()->Value();
12281226

12291227
bool use_bigint = args[1]->IsTrue();
1230-
FSReqBase* req_wrap_async = GetReqWrap(args, 2, use_bigint);
1231-
if (req_wrap_async != nullptr) { // fstat(fd, use_bigint, req)
1228+
if (argc > 2) { // fstat(fd, use_bigint, req)
1229+
FSReqBase* req_wrap_async = GetReqWrap(args, 2, use_bigint);
12321230
FS_ASYNC_TRACE_BEGIN0(UV_FS_FSTAT, req_wrap_async)
12331231
AsyncCall(env, req_wrap_async, args, "fstat", UTF8, AfterStat,
12341232
uv_fs_fstat, fd);
1235-
} else { // fstat(fd, use_bigint, undefined, ctx)
1236-
CHECK_EQ(argc, 4);
1237-
FSReqWrapSync req_wrap_sync;
1233+
} else { // fstat(fd, use_bigint)
1234+
FSReqWrapSync req_wrap_sync("fstat");
12381235
FS_SYNC_TRACE_BEGIN(fstat);
1239-
int err = SyncCall(env, args[3], &req_wrap_sync, "fstat", uv_fs_fstat, fd);
1236+
int err = SyncCallAndThrowOnError(env, &req_wrap_sync, uv_fs_fstat, fd);
12401237
FS_SYNC_TRACE_END(fstat);
1241-
if (err != 0) {
1242-
return; // error info is in ctx
1238+
if (is_uv_error(err)) {
1239+
return;
12431240
}
12441241

12451242
Local<Value> arr = FillGlobalStatsArray(binding_data, use_bigint,

‎typings/internalBinding/fs.d.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -91,9 +91,9 @@ declare namespace InternalFSBinding {
9191
function fstat(fd: number, useBigint: boolean, req: FSReqCallback<Float64Array | BigUint64Array>): void;
9292
function fstat(fd: number, useBigint: true, req: FSReqCallback<BigUint64Array>): void;
9393
function fstat(fd: number, useBigint: false, req: FSReqCallback<Float64Array>): void;
94-
function fstat(fd: number, useBigint: boolean, req: undefined, ctx: FSSyncContext): Float64Array | BigUint64Array;
95-
function fstat(fd: number, useBigint: true, req: undefined, ctx: FSSyncContext): BigUint64Array;
96-
function fstat(fd: number, useBigint: false, req: undefined, ctx: FSSyncContext): Float64Array;
94+
function fstat(fd: number, useBigint: boolean): Float64Array | BigUint64Array;
95+
function fstat(fd: number, useBigint: true): BigUint64Array;
96+
function fstat(fd: number, useBigint: false): Float64Array;
9797
function fstat(fd: number, useBigint: boolean, usePromises: typeof kUsePromises): Promise<Float64Array | BigUint64Array>;
9898
function fstat(fd: number, useBigint: true, usePromises: typeof kUsePromises): Promise<BigUint64Array>;
9999
function fstat(fd: number, useBigint: false, usePromises: typeof kUsePromises): Promise<Float64Array>;
@@ -124,9 +124,9 @@ declare namespace InternalFSBinding {
124124
function lstat(path: StringOrBuffer, useBigint: boolean, req: FSReqCallback<Float64Array | BigUint64Array>): void;
125125
function lstat(path: StringOrBuffer, useBigint: true, req: FSReqCallback<BigUint64Array>): void;
126126
function lstat(path: StringOrBuffer, useBigint: false, req: FSReqCallback<Float64Array>): void;
127-
function lstat(path: StringOrBuffer, useBigint: boolean, req: undefined, ctx: FSSyncContext): Float64Array | BigUint64Array;
128-
function lstat(path: StringOrBuffer, useBigint: true, req: undefined, ctx: FSSyncContext): BigUint64Array;
129-
function lstat(path: StringOrBuffer, useBigint: false, req: undefined, ctx: FSSyncContext): Float64Array;
127+
function lstat(path: StringOrBuffer, useBigint: boolean): Float64Array | BigUint64Array;
128+
function lstat(path: StringOrBuffer, useBigint: true): BigUint64Array;
129+
function lstat(path: StringOrBuffer, useBigint: false): Float64Array;
130130
function lstat(path: StringOrBuffer, useBigint: boolean, usePromises: typeof kUsePromises): Promise<Float64Array | BigUint64Array>;
131131
function lstat(path: StringOrBuffer, useBigint: true, usePromises: typeof kUsePromises): Promise<BigUint64Array>;
132132
function lstat(path: StringOrBuffer, useBigint: false, usePromises: typeof kUsePromises): Promise<Float64Array>;

0 commit comments

Comments
 (0)