Skip to content

Commit

Permalink
fs: add bufferSize option to fs.opendir()
Browse files Browse the repository at this point in the history
Add an option that controls the size of the internal
buffer.

Fixes: #29941
  • Loading branch information
addaleax committed Oct 25, 2019
1 parent 5fa55ee commit ac9b36b
Show file tree
Hide file tree
Showing 5 changed files with 64 additions and 18 deletions.
12 changes: 7 additions & 5 deletions benchmark/fs/bench-opendir.js
Expand Up @@ -7,22 +7,24 @@ 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();

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) => {
Expand All @@ -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();
Expand Down
21 changes: 21 additions & 0 deletions doc/api/fs.md
Expand Up @@ -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: REPLACME

This comment has been minimized.

Copy link
@z1c0

z1c0 Oct 25, 2019

Typo? Missing an E (also for the next 2 ocurrences)

pr-url: ???
description: The `bufferSize` option was introduced.
-->

* `path` {string|Buffer|URL}
* `options` {Object}
* `encoding` {string|null} **Default:** `'utf8'`
* `bufferSize` {number} Size of the internal buffer when reading
directory entries. Higher values lead to better performance but higher
memory usage. **Default:** `32`
* `callback` {Function}
* `err` {Error}
* `dir` {fs.Dir}
Expand All @@ -2645,11 +2652,18 @@ directory and subsequent read operations.
## fs.opendirSync(path\[, options\])
<!-- YAML
added: v12.12.0
changes:
- version: REPLACME
pr-url: ???
description: The `bufferSize` option was introduced.
-->

* `path` {string|Buffer|URL}
* `options` {Object}
* `encoding` {string|null} **Default:** `'utf8'`
* `bufferSize` {number} Size of the internal buffer when reading
directory entries. Higher values lead to better performance but higher
memory usage. **Default:** `32`
* Returns: {fs.Dir}

Synchronously open a directory. See opendir(3).
Expand Down Expand Up @@ -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: REPLACME
pr-url: ???
description: The `bufferSize` option was introduced.
-->

* `path` {string|Buffer|URL}
* `options` {Object}
* `encoding` {string|null} **Default:** `'utf8'`
* `bufferSize` {number} Size of the internal buffer when reading
directory entries. Higher values lead to better performance but higher
memory usage. **Default:** `32`
* Returns: {Promise} containing {fs.Dir}

Asynchronously open a directory. See opendir(3).
Expand Down
19 changes: 16 additions & 3 deletions lib/internal/fs/dir.js
Expand Up @@ -9,6 +9,7 @@ const {
codes: {
ERR_DIR_CLOSED,
ERR_INVALID_CALLBACK,
ERR_INVALID_OPT_VALUE,
ERR_MISSING_ARGS
}
} = require('internal/errors');
Expand Down Expand Up @@ -39,9 +40,19 @@ class Dir {
this[kDirPath] = path;
this[kDirClosed] = false;

this[kDirOptions] = getOptions(options, {
encoding: 'utf8'
});
this[kDirOptions] = {
bufferSize: 32,
...getOptions(options, {
encoding: 'utf8'
})
};

if (typeof this[kDirOptions].bufferSize !== 'number' ||
!Number.isInteger(this[kDirOptions].bufferSize) ||
this[kDirOptions].bufferSize <= 0) {
throw new ERR_INVALID_OPT_VALUE('bufferSize',
this[kDirOptions].bufferSize);
}

this[kDirReadPromisified] =
internalUtil.promisify(this[kDirReadImpl]).bind(this, false);
Expand Down Expand Up @@ -88,6 +99,7 @@ class Dir {

this[kDirHandle].read(
this[kDirOptions].encoding,
this[kDirOptions].bufferSize,
req
);
}
Expand All @@ -105,6 +117,7 @@ class Dir {
const ctx = { path: this[kDirPath] };
const result = this[kDirHandle].read(
this[kDirOptions].encoding,
this[kDirOptions].bufferSize,
undefined,
ctx
);
Expand Down
26 changes: 18 additions & 8 deletions src/node_dir.cc
Expand Up @@ -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;
Expand All @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down
4 changes: 2 additions & 2 deletions src/node_dir.h
Expand Up @@ -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;
};
Expand Down

0 comments on commit ac9b36b

Please sign in to comment.