archive/node
1
0
Fork 0
mirror of https://github.com/nodejs/node.git synced 2025-08-15 13:48:44 +02:00
Code Issues Projects Releases Packages Wiki Activity Actions 1

fs: improve error perf of sync lstat+fstat

Browse source 
PR-URL: https://github.com/nodejs/node/pull/49868
Refs: https://github.com/nodejs/performance/issues/106
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
This commit is contained in:
CanadaHonk 2023-11-23 00:25:15 +00:00 • committed by GitHub
parent 5dd7ba66ad
commit ea88a3e1f2
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 79 additions and 77 deletions
Download patch file Download diff file Expand all files Collapse all files

22
benchmark/fs/bench-statSync-failure.js
View file

@ -5,21 +5,29 @@ const fs = require('fs');
const path = require('path');
const bench = common.createBenchmark(main, {
n: [1e6],
statSyncType: ['throw', 'noThrow'],
n: [1e4],
statSyncType: ['fstatSync', 'lstatSync', 'statSync'],
throwType: [ 'throw', 'noThrow' ],
}, {
// fstatSync does not support throwIfNoEntry
combinationFilter: ({ statSyncType, throwType }) => !(statSyncType === 'fstatSync' && throwType === 'noThrow'),
});
function main({ n, statSyncType }) {
const arg = path.join(__dirname, 'non.existent');
function main({ n, statSyncType, throwType }) {
const arg = (statSyncType === 'fstatSync' ?
(1 << 30) :
path.join(__dirname, 'non.existent'));
const fn = fs[statSyncType];
bench.start();
for (let i = 0; i < n; i++) {
if (statSyncType === 'noThrow') {
fs.statSync(arg, { throwIfNoEntry: false });
if (throwType === 'noThrow') {
fn(arg, { throwIfNoEntry: false });
} else {
try {
fs.statSync(arg);
fn(arg);
} catch {
// Continue regardless of error.
}

2
benchmark/fs/bench-statSync.js
View file

@ -4,7 +4,7 @@ const common = require('../common');
const fs = require('fs');
const bench = common.createBenchmark(main, {
n: [1e6],
n: [1e4],
statSyncType: ['fstatSync', 'lstatSync', 'statSync'],
});

68
lib/fs.js
View file

@ -74,8 +74,6 @@ const {
ERR_INVALID_ARG_VALUE,
},
AbortError,
uvErrmapGet,
UVException,
} = require('internal/errors');
const {
@ -402,11 +400,9 @@ function readFile(path, options, callback) {
}
function tryStatSync(fd, isUserFd) {
const ctx = {};
const stats = binding.fstat(fd, false, undefined, ctx);
if (ctx.errno !== undefined && !isUserFd) {
const stats = binding.fstat(fd, false, undefined, true /* shouldNotThrow */);
if (stats === undefined && !isUserFd) {
fs.closeSync(fd);
throw new UVException(ctx);
}
return stats;
}
@ -1616,19 +1612,6 @@ function statfs(path, options = { bigint: false }, callback) {
binding.statfs(pathModule.toNamespacedPath(path), options.bigint, req);
}
function hasNoEntryError(ctx) {
if (ctx.errno) {
const uvErr = uvErrmapGet(ctx.errno);
return uvErr?.[0] === 'ENOENT';
}
if (ctx.error) {
return ctx.error.code === 'ENOENT';
}
return false;
}
/**
* Synchronously retrieves the `fs.Stats` for
* the file descriptor.
@ -1636,13 +1619,14 @@ function hasNoEntryError(ctx) {
* @param {{
* bigint?: boolean;
* }} [options]
* @returns {Stats}
* @returns {Stats | undefined}
*/
function fstatSync(fd, options = { bigint: false }) {
fd = getValidatedFd(fd);
const ctx = { fd };
const stats = binding.fstat(fd, options.bigint, undefined, ctx);
handleErrorFromBinding(ctx);
const stats = binding.fstat(fd, options.bigint, undefined, false);
if (stats === undefined) {
return;
}
return getStatsFromBinding(stats);
}
@ -1654,17 +1638,20 @@ function fstatSync(fd, options = { bigint: false }) {
* bigint?: boolean;
* throwIfNoEntry?: boolean;
* }} [options]
* @returns {Stats}
* @returns {Stats | undefined}
*/
function lstatSync(path, options = { bigint: false, throwIfNoEntry: true }) {
path = getValidatedPath(path);
const ctx = { path };
const stats = binding.lstat(pathModule.toNamespacedPath(path),
options.bigint, undefined, ctx);
if (options.throwIfNoEntry === false && hasNoEntryError(ctx)) {
return undefined;
const stats = binding.lstat(
pathModule.toNamespacedPath(path),
options.bigint,
undefined,
options.throwIfNoEntry,
);
if (stats === undefined) {
return;
}
handleErrorFromBinding(ctx);
return getStatsFromBinding(stats);
}
@ -2667,9 +2654,10 @@ function realpathSync(p, options) {
// On windows, check that the root exists. On unix there is no need.
if (isWindows) {
const ctx = { path: base };
binding.lstat(pathModule.toNamespacedPath(base), false, undefined, ctx);
handleErrorFromBinding(ctx);
const out = binding.lstat(pathModule.toNamespacedPath(base), false, undefined, true /* throwIfNoEntry */);
if (out === undefined) {
return;
}
knownHard.add(base);
}
@ -2709,9 +2697,10 @@ function realpathSync(p, options) {
// for our internal use.
const baseLong = pathModule.toNamespacedPath(base);
const ctx = { path: base };
const stats = binding.lstat(baseLong, true, undefined, ctx);
handleErrorFromBinding(ctx);
const stats = binding.lstat(baseLong, true, undefined, true /* throwIfNoEntry */);
if (stats === undefined) {
return;
}
if (!isFileType(stats, S_IFLNK)) {
knownHard.add(base);
@ -2750,9 +2739,10 @@ function realpathSync(p, options) {
// On windows, check that the root exists. On unix there is no need.
if (isWindows && !knownHard.has(base)) {
const ctx = { path: base };
binding.lstat(pathModule.toNamespacedPath(base), false, undefined, ctx);
handleErrorFromBinding(ctx);
const out = binding.lstat(pathModule.toNamespacedPath(base), false, undefined, true /* throwIfNoEntry */);
if (out === undefined) {
return;
}
knownHard.add(base);
}
}

43
src/node_file.cc
View file

@ -1118,21 +1118,26 @@ static void LStat(const FunctionCallbackInfo<Value>& args) {
CHECK_NOT_NULL(*path);
bool use_bigint = args[1]->IsTrue();
FSReqBase* req_wrap_async = GetReqWrap(args, 2, use_bigint);
if (req_wrap_async != nullptr) { // lstat(path, use_bigint, req)
if (!args[2]->IsUndefined()) { // lstat(path, use_bigint, req)
FSReqBase* req_wrap_async = GetReqWrap(args, 2, use_bigint);
FS_ASYNC_TRACE_BEGIN1(
UV_FS_LSTAT, req_wrap_async, "path", TRACE_STR_COPY(*path))
AsyncCall(env, req_wrap_async, args, "lstat", UTF8, AfterStat,
uv_fs_lstat, *path);
} else { // lstat(path, use_bigint, undefined, ctx)
CHECK_EQ(argc, 4);
FSReqWrapSync req_wrap_sync;
} else { // lstat(path, use_bigint, undefined, throw_if_no_entry)
bool do_not_throw_if_no_entry = args[3]->IsFalse();
FSReqWrapSync req_wrap_sync("lstat", *path);
FS_SYNC_TRACE_BEGIN(lstat);
int err = SyncCall(env, args[3], &req_wrap_sync, "lstat", uv_fs_lstat,
*path);
int result;
if (do_not_throw_if_no_entry) {
result = SyncCallAndThrowIf(
is_uv_error_except_no_entry, env, &req_wrap_sync, uv_fs_lstat, *path);
} else {
result = SyncCallAndThrowOnError(env, &req_wrap_sync, uv_fs_lstat, *path);
}
FS_SYNC_TRACE_END(lstat);
if (err != 0) {
return; // error info is in ctx
if (is_uv_error(result)) {
return;
}
Local<Value> arr = FillGlobalStatsArray(binding_data, use_bigint,
@ -1153,19 +1158,23 @@ static void FStat(const FunctionCallbackInfo<Value>& args) {
int fd = args[0].As<Int32>()->Value();
bool use_bigint = args[1]->IsTrue();
FSReqBase* req_wrap_async = GetReqWrap(args, 2, use_bigint);
if (req_wrap_async != nullptr) { // fstat(fd, use_bigint, req)
if (!args[2]->IsUndefined()) { // fstat(fd, use_bigint, req)
FSReqBase* req_wrap_async = GetReqWrap(args, 2, use_bigint);
FS_ASYNC_TRACE_BEGIN0(UV_FS_FSTAT, req_wrap_async)
AsyncCall(env, req_wrap_async, args, "fstat", UTF8, AfterStat,
uv_fs_fstat, fd);
} else { // fstat(fd, use_bigint, undefined, ctx)
CHECK_EQ(argc, 4);
FSReqWrapSync req_wrap_sync;
} else { // fstat(fd, use_bigint, undefined, do_not_throw_error)
bool do_not_throw_error = args[2]->IsTrue();
const auto should_throw = [do_not_throw_error](int result) {
return is_uv_error(result) && !do_not_throw_error;
};
FSReqWrapSync req_wrap_sync("fstat");
FS_SYNC_TRACE_BEGIN(fstat);
int err = SyncCall(env, args[3], &req_wrap_sync, "fstat", uv_fs_fstat, fd);
int err =
SyncCallAndThrowIf(should_throw, env, &req_wrap_sync, uv_fs_fstat, fd);
FS_SYNC_TRACE_END(fstat);
if (err != 0) {
return; // error info is in ctx
if (is_uv_error(err)) {
return;
}
Local<Value> arr = FillGlobalStatsArray(binding_data, use_bigint,

9
test/parallel/test-fs-sync-fd-leak.js
View file

@ -25,7 +25,6 @@ require('../common');
const assert = require('assert');
const fs = require('fs');
const { internalBinding } = require('internal/test/binding');
const { UV_EBADF } = internalBinding('uv');
// Ensure that (read|write|append)FileSync() closes the file descriptor
fs.openSync = function() {
@ -42,15 +41,11 @@ fs.writeSync = function() {
throw new Error('BAM');
};
internalBinding('fs').fstat = function(fd, bigint, _, ctx) {
ctx.errno = UV_EBADF;
ctx.syscall = 'fstat';
internalBinding('fs').fstat = function() {
throw new Error('EBADF: bad file descriptor, fstat');
};
let close_called = 0;
ensureThrows(function() {
fs.readFileSync('dummy');
}, 'EBADF: bad file descriptor, fstat');
ensureThrows(function() {
fs.writeFileSync('dummy', 'xxx');
}, 'BAM');

12
typings/internalBinding/fs.d.ts vendored
View file

@ -92,9 +92,9 @@ declare namespace InternalFSBinding {
function fstat(fd: number, useBigint: boolean, req: FSReqCallback<Float64Array | BigUint64Array>): void;
function fstat(fd: number, useBigint: true, req: FSReqCallback<BigUint64Array>): void;
function fstat(fd: number, useBigint: false, req: FSReqCallback<Float64Array>): void;
function fstat(fd: number, useBigint: boolean, req: undefined, ctx: FSSyncContext): Float64Array | BigUint64Array;
function fstat(fd: number, useBigint: true, req: undefined, ctx: FSSyncContext): BigUint64Array;
function fstat(fd: number, useBigint: false, req: undefined, ctx: FSSyncContext): Float64Array;
function fstat(fd: number, useBigint: boolean, req: undefined, shouldNotThrow: boolean): Float64Array | BigUint64Array;
function fstat(fd: number, useBigint: true, req: undefined, shouldNotThrow: boolean): BigUint64Array;
function fstat(fd: number, useBigint: false, req: undefined, shouldNotThrow: boolean): Float64Array;
function fstat(fd: number, useBigint: boolean, usePromises: typeof kUsePromises): Promise<Float64Array | BigUint64Array>;
function fstat(fd: number, useBigint: true, usePromises: typeof kUsePromises): Promise<BigUint64Array>;
function fstat(fd: number, useBigint: false, usePromises: typeof kUsePromises): Promise<Float64Array>;
@ -126,9 +126,9 @@ declare namespace InternalFSBinding {
function lstat(path: StringOrBuffer, useBigint: boolean, req: FSReqCallback<Float64Array | BigUint64Array>): void;
function lstat(path: StringOrBuffer, useBigint: true, req: FSReqCallback<BigUint64Array>): void;
function lstat(path: StringOrBuffer, useBigint: false, req: FSReqCallback<Float64Array>): void;
function lstat(path: StringOrBuffer, useBigint: boolean, req: undefined, ctx: FSSyncContext): Float64Array | BigUint64Array;
function lstat(path: StringOrBuffer, useBigint: true, req: undefined, ctx: FSSyncContext): BigUint64Array;
function lstat(path: StringOrBuffer, useBigint: false, req: undefined, ctx: FSSyncContext): Float64Array;
function lstat(path: StringOrBuffer, useBigint: boolean, req: undefined, throwIfNoEntry: boolean): Float64Array | BigUint64Array;
function lstat(path: StringOrBuffer, useBigint: true, req: undefined, throwIfNoEntry: boolean): BigUint64Array;
function lstat(path: StringOrBuffer, useBigint: false, req: undefined, throwIfNoEntry: boolean): Float64Array;
function lstat(path: StringOrBuffer, useBigint: boolean, usePromises: typeof kUsePromises): Promise<Float64Array | BigUint64Array>;
function lstat(path: StringOrBuffer, useBigint: true, usePromises: typeof kUsePromises): Promise<BigUint64Array>;
function lstat(path: StringOrBuffer, useBigint: false, usePromises: typeof kUsePromises): Promise<Float64Array>;