summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--benchmark/fs/bench-opendir.js12
-rw-r--r--doc/api/fs.md21
-rw-r--r--lib/internal/fs/dir.js16
-rw-r--r--src/node_dir.cc26
-rw-r--r--src/node_dir.h4
-rw-r--r--test/benchmark/test-benchmark-fs.js1
-rw-r--r--test/parallel/test-fs-opendir.js23
7 files changed, 85 insertions, 18 deletions
diff --git a/benchmark/fs/bench-opendir.js b/benchmark/fs/bench-opendir.js
index 419c3a231a..20e178d9cc 100644
--- a/benchmark/fs/bench-opendir.js
+++ b/benchmark/fs/bench-opendir.js
@@ -7,10 +7,11 @@ const path = require('path');
const bench = common.createBenchmark(main, {
n: [100],
dir: [ 'lib', 'test/parallel'],
- mode: [ 'async', 'sync', 'callback' ]
+ mode: [ 'async', 'sync', 'callback' ],
+ bufferSize: [ 4, 32, 1024 ]
});
-async function main({ n, dir, mode }) {
+async function main({ n, dir, mode, bufferSize }) {
const fullPath = path.resolve(__dirname, '../../', dir);
bench.start();
@@ -18,11 +19,12 @@ async function main({ n, dir, mode }) {
let counter = 0;
for (let i = 0; i < n; i++) {
if (mode === 'async') {
+ const dir = await fs.promises.opendir(fullPath, { bufferSize });
// eslint-disable-next-line no-unused-vars
- for await (const entry of await fs.promises.opendir(fullPath))
+ for await (const entry of dir)
counter++;
} else if (mode === 'callback') {
- const dir = await fs.promises.opendir(fullPath);
+ const dir = await fs.promises.opendir(fullPath, { bufferSize });
await new Promise((resolve, reject) => {
function read() {
dir.read((err, entry) => {
@@ -40,7 +42,7 @@ async function main({ n, dir, mode }) {
read();
});
} else {
- const dir = fs.opendirSync(fullPath);
+ const dir = fs.opendirSync(fullPath, { bufferSize });
while (dir.readSync() !== null)
counter++;
dir.closeSync();
diff --git a/doc/api/fs.md b/doc/api/fs.md
index 78b7b3af2f..7f6c815b9b 100644
--- a/doc/api/fs.md
+++ b/doc/api/fs.md
@@ -2625,11 +2625,18 @@ Functions based on `fs.open()` exhibit this behavior as well:
## fs.opendir(path\[, options\], callback)
<!-- YAML
added: v12.12.0
+changes:
+ - version: REPLACEME
+ pr-url: https://github.com/nodejs/node/pull/30114
+ description: The `bufferSize` option was introduced.
-->
* `path` {string|Buffer|URL}
* `options` {Object}
* `encoding` {string|null} **Default:** `'utf8'`
+ * `bufferSize` {number} Number of directory entries that are buffered
+ internally when reading from the directory. Higher values lead to better
+ performance but higher memory usage. **Default:** `32`
* `callback` {Function}
* `err` {Error}
* `dir` {fs.Dir}
@@ -2645,11 +2652,18 @@ directory and subsequent read operations.
## fs.opendirSync(path\[, options\])
<!-- YAML
added: v12.12.0
+changes:
+ - version: REPLACEME
+ pr-url: https://github.com/nodejs/node/pull/30114
+ description: The `bufferSize` option was introduced.
-->
* `path` {string|Buffer|URL}
* `options` {Object}
* `encoding` {string|null} **Default:** `'utf8'`
+ * `bufferSize` {number} Number of directory entries that are buffered
+ internally when reading from the directory. Higher values lead to better
+ performance but higher memory usage. **Default:** `32`
* Returns: {fs.Dir}
Synchronously open a directory. See opendir(3).
@@ -4829,11 +4843,18 @@ a colon, Node.js will open a file system stream, as described by
### fsPromises.opendir(path\[, options\])
<!-- YAML
added: v12.12.0
+changes:
+ - version: REPLACEME
+ pr-url: https://github.com/nodejs/node/pull/30114
+ description: The `bufferSize` option was introduced.
-->
* `path` {string|Buffer|URL}
* `options` {Object}
* `encoding` {string|null} **Default:** `'utf8'`
+ * `bufferSize` {number} Number of directory entries that are buffered
+ internally when reading from the directory. Higher values lead to better
+ performance but higher memory usage. **Default:** `32`
* Returns: {Promise} containing {fs.Dir}
Asynchronously open a directory. See opendir(3).
diff --git a/lib/internal/fs/dir.js b/lib/internal/fs/dir.js
index fedd7ff8ed..90ab31fe5a 100644
--- a/lib/internal/fs/dir.js
+++ b/lib/internal/fs/dir.js
@@ -21,6 +21,9 @@ const {
getValidatedPath,
handleErrorFromBinding
} = require('internal/fs/utils');
+const {
+ validateUint32
+} = require('internal/validators');
const kDirHandle = Symbol('kDirHandle');
const kDirPath = Symbol('kDirPath');
@@ -39,9 +42,14 @@ class Dir {
this[kDirPath] = path;
this[kDirClosed] = false;
- this[kDirOptions] = getOptions(options, {
- encoding: 'utf8'
- });
+ this[kDirOptions] = {
+ bufferSize: 32,
+ ...getOptions(options, {
+ encoding: 'utf8'
+ })
+ };
+
+ validateUint32(this[kDirOptions].bufferSize, 'options.bufferSize', true);
this[kDirReadPromisified] =
internalUtil.promisify(this[kDirReadImpl]).bind(this, false);
@@ -88,6 +96,7 @@ class Dir {
this[kDirHandle].read(
this[kDirOptions].encoding,
+ this[kDirOptions].bufferSize,
req
);
}
@@ -105,6 +114,7 @@ class Dir {
const ctx = { path: this[kDirPath] };
const result = this[kDirHandle].read(
this[kDirOptions].encoding,
+ this[kDirOptions].bufferSize,
undefined,
ctx
);
diff --git a/src/node_dir.cc b/src/node_dir.cc
index 382f00f56e..ffef27f452 100644
--- a/src/node_dir.cc
+++ b/src/node_dir.cc
@@ -36,6 +36,7 @@ using v8::Isolate;
using v8::Local;
using v8::MaybeLocal;
using v8::Null;
+using v8::Number;
using v8::Object;
using v8::ObjectTemplate;
using v8::String;
@@ -59,8 +60,8 @@ DirHandle::DirHandle(Environment* env, Local<Object> obj, uv_dir_t* dir)
dir_(dir) {
MakeWeak();
- dir_->nentries = arraysize(dirents_);
- dir_->dirents = dirents_;
+ dir_->nentries = 0;
+ dir_->dirents = nullptr;
}
DirHandle* DirHandle::New(Environment* env, uv_dir_t* dir) {
@@ -230,22 +231,31 @@ void DirHandle::Read(const FunctionCallbackInfo<Value>& args) {
Isolate* isolate = env->isolate();
const int argc = args.Length();
- CHECK_GE(argc, 2);
+ CHECK_GE(argc, 3);
const enum encoding encoding = ParseEncoding(isolate, args[0], UTF8);
DirHandle* dir;
ASSIGN_OR_RETURN_UNWRAP(&dir, args.Holder());
- FSReqBase* req_wrap_async = GetReqWrap(env, args[1]);
- if (req_wrap_async != nullptr) { // dir.read(encoding, req)
+ CHECK(args[1]->IsNumber());
+ uint64_t buffer_size = args[1].As<Number>()->Value();
+
+ if (buffer_size != dir->dirents_.size()) {
+ dir->dirents_.resize(buffer_size);
+ dir->dir_->nentries = buffer_size;
+ dir->dir_->dirents = dir->dirents_.data();
+ }
+
+ FSReqBase* req_wrap_async = GetReqWrap(env, args[2]);
+ if (req_wrap_async != nullptr) { // dir.read(encoding, bufferSize, req)
AsyncCall(env, req_wrap_async, args, "readdir", encoding,
AfterDirRead, uv_fs_readdir, dir->dir());
- } else { // dir.read(encoding, undefined, ctx)
- CHECK_EQ(argc, 3);
+ } else { // dir.read(encoding, bufferSize, undefined, ctx)
+ CHECK_EQ(argc, 4);
FSReqWrapSync req_wrap_sync;
FS_DIR_SYNC_TRACE_BEGIN(readdir);
- int err = SyncCall(env, args[2], &req_wrap_sync, "readdir", uv_fs_readdir,
+ int err = SyncCall(env, args[3], &req_wrap_sync, "readdir", uv_fs_readdir,
dir->dir());
FS_DIR_SYNC_TRACE_END(readdir);
if (err < 0) {
diff --git a/src/node_dir.h b/src/node_dir.h
index 03e4a06efc..ae6d0eb170 100644
--- a/src/node_dir.h
+++ b/src/node_dir.h
@@ -45,8 +45,8 @@ class DirHandle : public AsyncWrap {
void GCClose();
uv_dir_t* dir_;
- // Up to 32 directory entries are read through a single libuv call.
- uv_dirent_t dirents_[32];
+ // Multiple entries are read through a single libuv call.
+ std::vector<uv_dirent_t> dirents_;
bool closing_ = false;
bool closed_ = false;
};
diff --git a/test/benchmark/test-benchmark-fs.js b/test/benchmark/test-benchmark-fs.js
index 3168e6f0a0..cf38240723 100644
--- a/test/benchmark/test-benchmark-fs.js
+++ b/test/benchmark/test-benchmark-fs.js
@@ -7,6 +7,7 @@ const tmpdir = require('../common/tmpdir');
tmpdir.refresh();
runBenchmark('fs', [
+ 'bufferSize=32',
'concurrent=1',
'dir=.github',
'dur=0.1',
diff --git a/test/parallel/test-fs-opendir.js b/test/parallel/test-fs-opendir.js
index f2c5d03345..05fded527f 100644
--- a/test/parallel/test-fs-opendir.js
+++ b/test/parallel/test-fs-opendir.js
@@ -182,3 +182,26 @@ async function doAsyncIterThrowTest() {
await assert.rejects(async () => dir.read(), dirclosedError);
}
doAsyncIterThrowTest().then(common.mustCall());
+
+// Check error thrown on invalid values of bufferSize
+for (const bufferSize of [-1, 0, 0.5, 1.5, Infinity, NaN]) {
+ assert.throws(
+ () => fs.opendirSync(testDir, { bufferSize }),
+ {
+ code: 'ERR_OUT_OF_RANGE'
+ });
+}
+for (const bufferSize of ['', '1', null]) {
+ assert.throws(
+ () => fs.opendirSync(testDir, { bufferSize }),
+ {
+ code: 'ERR_INVALID_ARG_TYPE'
+ });
+}
+
+// Check that passing a positive integer as bufferSize works
+{
+ const dir = fs.opendirSync(testDir, { bufferSize: 1024 });
+ assertDirent(dir.readSync());
+ dir.close();
+}