From 4f8e64d1648c32c7f21f8a3e0eb13721cdbb2b26 Mon Sep 17 00:00:00 2001 From: Yagiz Nizipli Date: Mon, 18 Sep 2023 10:45:55 -0400 Subject: [PATCH 1/2] fs: improve error performance of `opendirSync` --- benchmark/fs/bench-opendirSync.js | 38 +++++++++++++++++++++++++++++++ lib/internal/fs/dir.js | 11 ++------- src/node_dir.cc | 28 +++++++++++++++++++++++ 3 files changed, 68 insertions(+), 9 deletions(-) create mode 100644 benchmark/fs/bench-opendirSync.js diff --git a/benchmark/fs/bench-opendirSync.js b/benchmark/fs/bench-opendirSync.js new file mode 100644 index 00000000000000..92b02e928e24f7 --- /dev/null +++ b/benchmark/fs/bench-opendirSync.js @@ -0,0 +1,38 @@ +'use strict'; + +const common = require('../common'); +const fs = require('fs'); +const tmpdir = require('../../test/common/tmpdir'); +tmpdir.refresh(); + +const bench = common.createBenchmark(main, { + type: ['existing', 'non-existing'], + bufferSize: [ 4, 32, 128 ], + n: [1e4], +}); + +function main({ n, type, bufferSize }) { + let path; + + switch (type) { + case 'existing': + path = 'lib'; + break; + case 'non-existing': + path = tmpdir.resolve(`.non-existing-file-${process.pid}`); + break; + default: + new Error('Invalid type'); + } + + bench.start(); + for (let i = 0; i < n; i++) { + try { + const dir = fs.opendirSync(path, { bufferSize }); + dir.closeSync(); + } catch { + // do nothing + } + } + bench.end(n); +} diff --git a/lib/internal/fs/dir.js b/lib/internal/fs/dir.js index a443532d35d3ed..1118ff5f674915 100644 --- a/lib/internal/fs/dir.js +++ b/lib/internal/fs/dir.js @@ -322,18 +322,11 @@ function opendir(path, options, callback) { function opendirSync(path, options) { path = getValidatedPath(path); - options = getOptions(options, { - encoding: 'utf8', - }); + options = getOptions(options, { encoding: 'utf8' }); - const ctx = { path }; - const handle = dirBinding.opendir( + const handle = dirBinding.opendirSync( pathModule.toNamespacedPath(path), - options.encoding, - undefined, - ctx, ); - handleErrorFromBinding(ctx); return new Dir(handle, path, options); } diff --git a/src/node_dir.cc b/src/node_dir.cc index 8f93f189cfbe27..10cde6067899c7 100644 --- a/src/node_dir.cc +++ b/src/node_dir.cc @@ -397,6 +397,32 @@ static void OpenDir(const FunctionCallbackInfo& args) { } } +static void OpenDirSync(const FunctionCallbackInfo& args) { + Environment* env = Environment::GetCurrent(args); + Isolate* isolate = env->isolate(); + + CHECK_GE(args.Length(), 1); + + BufferValue path(isolate, args[0]); + CHECK_NOT_NULL(*path); + THROW_IF_INSUFFICIENT_PERMISSIONS( + env, permission::PermissionScope::kFileSystemRead, path.ToStringView()); + + uv_fs_t req; + auto make = OnScopeLeave([&req]() { uv_fs_req_cleanup(&req); }); + FS_DIR_SYNC_TRACE_BEGIN(opendir); + int err = uv_fs_opendir(nullptr, &req, *path, nullptr); + FS_DIR_SYNC_TRACE_END(opendir); + if (err < 0) { + return env->ThrowUVException(err, "opendir"); + } + + uv_dir_t* dir = static_cast(req.ptr); + DirHandle* handle = DirHandle::New(env, dir); + + args.GetReturnValue().Set(handle->object().As()); +} + void Initialize(Local target, Local unused, Local context, @@ -405,6 +431,7 @@ void Initialize(Local target, Isolate* isolate = env->isolate(); SetMethod(context, target, "opendir", OpenDir); + SetMethod(context, target, "opendirSync", OpenDirSync); // Create FunctionTemplate for DirHandle Local dir = NewFunctionTemplate(isolate, DirHandle::New); @@ -419,6 +446,7 @@ void Initialize(Local target, void RegisterExternalReferences(ExternalReferenceRegistry* registry) { registry->Register(OpenDir); + registry->Register(OpenDirSync); registry->Register(DirHandle::New); registry->Register(DirHandle::Read); registry->Register(DirHandle::Close); From 134eee9cf04afe86d5dc6b2f3aa2c42fe3d16438 Mon Sep 17 00:00:00 2001 From: Yagiz Nizipli Date: Mon, 18 Sep 2023 20:28:36 -0400 Subject: [PATCH 2/2] fixup! fs: improve error performance of `opendirSync` --- benchmark/fs/bench-opendirSync.js | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/benchmark/fs/bench-opendirSync.js b/benchmark/fs/bench-opendirSync.js index 92b02e928e24f7..206822db139ff7 100644 --- a/benchmark/fs/bench-opendirSync.js +++ b/benchmark/fs/bench-opendirSync.js @@ -2,24 +2,27 @@ const common = require('../common'); const fs = require('fs'); +const path = require('path'); const tmpdir = require('../../test/common/tmpdir'); tmpdir.refresh(); +const testFiles = fs.readdirSync('test', { withFileTypes: true }) + .filter((f) => f.isDirectory()) + .map((f) => path.join(f.path, f.name)); const bench = common.createBenchmark(main, { type: ['existing', 'non-existing'], - bufferSize: [ 4, 32, 128 ], - n: [1e4], + n: [1e3], }); -function main({ n, type, bufferSize }) { - let path; +function main({ n, type }) { + let files; switch (type) { case 'existing': - path = 'lib'; + files = testFiles; break; case 'non-existing': - path = tmpdir.resolve(`.non-existing-file-${process.pid}`); + files = [tmpdir.resolve(`.non-existing-file-${Date.now()}`)]; break; default: new Error('Invalid type'); @@ -27,11 +30,13 @@ function main({ n, type, bufferSize }) { bench.start(); for (let i = 0; i < n; i++) { - try { - const dir = fs.opendirSync(path, { bufferSize }); - dir.closeSync(); - } catch { - // do nothing + for (let j = 0; j < files.length; j++) { + try { + const dir = fs.opendirSync(files[j]); + dir.closeSync(); + } catch { + // do nothing + } } } bench.end(n);