Skip to content

Commit

Permalink
fs: improve error performance of opendirSync
Browse files Browse the repository at this point in the history
PR-URL: #49705
Refs: nodejs/performance#106
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
  • Loading branch information
anonrig authored Sep 21, 2023
1 parent b7d836e commit 571ecbf
Show file tree
Hide file tree
Showing 3 changed files with 73 additions and 9 deletions.
43 changes: 43 additions & 0 deletions benchmark/fs/bench-opendirSync.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
'use strict';

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'],
n: [1e3],
});

function main({ n, type }) {
let files;

switch (type) {
case 'existing':
files = testFiles;
break;
case 'non-existing':
files = [tmpdir.resolve(`.non-existing-file-${Date.now()}`)];
break;
default:
new Error('Invalid type');
}

bench.start();
for (let i = 0; i < n; i++) {
for (let j = 0; j < files.length; j++) {
try {
const dir = fs.opendirSync(files[j]);
dir.closeSync();
} catch {
// do nothing
}
}
}
bench.end(n);
}
11 changes: 2 additions & 9 deletions lib/internal/fs/dir.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
28 changes: 28 additions & 0 deletions src/node_dir.cc
Original file line number Diff line number Diff line change
Expand Up @@ -397,6 +397,32 @@ static void OpenDir(const FunctionCallbackInfo<Value>& args) {
}
}

static void OpenDirSync(const FunctionCallbackInfo<Value>& 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<uv_dir_t*>(req.ptr);
DirHandle* handle = DirHandle::New(env, dir);

args.GetReturnValue().Set(handle->object().As<Value>());
}

void Initialize(Local<Object> target,
Local<Value> unused,
Local<Context> context,
Expand All @@ -405,6 +431,7 @@ void Initialize(Local<Object> target,
Isolate* isolate = env->isolate();

SetMethod(context, target, "opendir", OpenDir);
SetMethod(context, target, "opendirSync", OpenDirSync);

// Create FunctionTemplate for DirHandle
Local<FunctionTemplate> dir = NewFunctionTemplate(isolate, DirHandle::New);
Expand All @@ -419,6 +446,7 @@ void Initialize(Local<Object> target,

void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
registry->Register(OpenDir);
registry->Register(OpenDirSync);
registry->Register(DirHandle::New);
registry->Register(DirHandle::Read);
registry->Register(DirHandle::Close);
Expand Down

0 comments on commit 571ecbf

Please sign in to comment.