Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit ba5683c

Browse filesBrowse files
addaleaxMylesBorins
addaleax
authored and
MylesBorins
committed
fs: add bufferSize option to fs.opendir()
Add an option that controls the size of the internal buffer. Fixes: #29941 PR-URL: #30114 Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
1 parent f3196db commit ba5683c
Copy full SHA for ba5683c

File tree

Expand file treeCollapse file tree

7 files changed

+85
-18
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

7 files changed

+85
-18
lines changed
Open diff view settings
Collapse file

‎benchmark/fs/bench-opendir.js‎

Copy file name to clipboardExpand all lines: benchmark/fs/bench-opendir.js
+7-5Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,22 +7,24 @@ const path = require('path');
77
const bench = common.createBenchmark(main, {
88
n: [100],
99
dir: [ 'lib', 'test/parallel'],
10-
mode: [ 'async', 'sync', 'callback' ]
10+
mode: [ 'async', 'sync', 'callback' ],
11+
bufferSize: [ 4, 32, 1024 ]
1112
});
1213

13-
async function main({ n, dir, mode }) {
14+
async function main({ n, dir, mode, bufferSize }) {
1415
const fullPath = path.resolve(__dirname, '../../', dir);
1516

1617
bench.start();
1718

1819
let counter = 0;
1920
for (let i = 0; i < n; i++) {
2021
if (mode === 'async') {
22+
const dir = await fs.promises.opendir(fullPath, { bufferSize });
2123
// eslint-disable-next-line no-unused-vars
22-
for await (const entry of await fs.promises.opendir(fullPath))
24+
for await (const entry of dir)
2325
counter++;
2426
} else if (mode === 'callback') {
25-
const dir = await fs.promises.opendir(fullPath);
27+
const dir = await fs.promises.opendir(fullPath, { bufferSize });
2628
await new Promise((resolve, reject) => {
2729
function read() {
2830
dir.read((err, entry) => {
@@ -40,7 +42,7 @@ async function main({ n, dir, mode }) {
4042
read();
4143
});
4244
} else {
43-
const dir = fs.opendirSync(fullPath);
45+
const dir = fs.opendirSync(fullPath, { bufferSize });
4446
while (dir.readSync() !== null)
4547
counter++;
4648
dir.closeSync();
Collapse file

‎doc/api/fs.md‎

Copy file name to clipboardExpand all lines: doc/api/fs.md
+21Lines changed: 21 additions & 0 deletions
  • Display the source diff
  • Display the rich diff
Original file line numberDiff line numberDiff line change
@@ -2625,11 +2625,18 @@ Functions based on `fs.open()` exhibit this behavior as well:
26252625
## `fs.opendir(path[, options], callback)`
26262626
<!-- YAML
26272627
added: v12.12.0
2628+
changes:
2629+
- version: REPLACEME
2630+
pr-url: https://github.com/nodejs/node/pull/30114
2631+
description: The `bufferSize` option was introduced.
26282632
-->
26292633

26302634
* `path` {string|Buffer|URL}
26312635
* `options` {Object}
26322636
* `encoding` {string|null} **Default:** `'utf8'`
2637+
* `bufferSize` {number} Number of directory entries that are buffered
2638+
internally when reading from the directory. Higher values lead to better
2639+
performance but higher memory usage. **Default:** `32`
26332640
* `callback` {Function}
26342641
* `err` {Error}
26352642
* `dir` {fs.Dir}
@@ -2645,11 +2652,18 @@ directory and subsequent read operations.
26452652
## `fs.opendirSync(path[, options])`
26462653
<!-- YAML
26472654
added: v12.12.0
2655+
changes:
2656+
- version: REPLACEME
2657+
pr-url: https://github.com/nodejs/node/pull/30114
2658+
description: The `bufferSize` option was introduced.
26482659
-->
26492660

26502661
* `path` {string|Buffer|URL}
26512662
* `options` {Object}
26522663
* `encoding` {string|null} **Default:** `'utf8'`
2664+
* `bufferSize` {number} Number of directory entries that are buffered
2665+
internally when reading from the directory. Higher values lead to better
2666+
performance but higher memory usage. **Default:** `32`
26532667
* Returns: {fs.Dir}
26542668

26552669
Synchronously open a directory. See opendir(3).
@@ -4829,11 +4843,18 @@ a colon, Node.js will open a file system stream, as described by
48294843
### `fsPromises.opendir(path[, options])`
48304844
<!-- YAML
48314845
added: v12.12.0
4846+
changes:
4847+
- version: REPLACEME
4848+
pr-url: https://github.com/nodejs/node/pull/30114
4849+
description: The `bufferSize` option was introduced.
48324850
-->
48334851

48344852
* `path` {string|Buffer|URL}
48354853
* `options` {Object}
48364854
* `encoding` {string|null} **Default:** `'utf8'`
4855+
* `bufferSize` {number} Number of directory entries that are buffered
4856+
internally when reading from the directory. Higher values lead to better
4857+
performance but higher memory usage. **Default:** `32`
48374858
* Returns: {Promise} containing {fs.Dir}
48384859

48394860
Asynchronously open a directory. See opendir(3).
Collapse file

‎lib/internal/fs/dir.js‎

Copy file name to clipboardExpand all lines: lib/internal/fs/dir.js
+13-3Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,9 @@ const {
2121
getValidatedPath,
2222
handleErrorFromBinding
2323
} = require('internal/fs/utils');
24+
const {
25+
validateUint32
26+
} = require('internal/validators');
2427

2528
const kDirHandle = Symbol('kDirHandle');
2629
const kDirPath = Symbol('kDirPath');
@@ -39,9 +42,14 @@ class Dir {
3942
this[kDirPath] = path;
4043
this[kDirClosed] = false;
4144

42-
this[kDirOptions] = getOptions(options, {
43-
encoding: 'utf8'
44-
});
45+
this[kDirOptions] = {
46+
bufferSize: 32,
47+
...getOptions(options, {
48+
encoding: 'utf8'
49+
})
50+
};
51+
52+
validateUint32(this[kDirOptions].bufferSize, 'options.bufferSize', true);
4553

4654
this[kDirReadPromisified] =
4755
internalUtil.promisify(this[kDirReadImpl]).bind(this, false);
@@ -88,6 +96,7 @@ class Dir {
8896

8997
this[kDirHandle].read(
9098
this[kDirOptions].encoding,
99+
this[kDirOptions].bufferSize,
91100
req
92101
);
93102
}
@@ -105,6 +114,7 @@ class Dir {
105114
const ctx = { path: this[kDirPath] };
106115
const result = this[kDirHandle].read(
107116
this[kDirOptions].encoding,
117+
this[kDirOptions].bufferSize,
108118
undefined,
109119
ctx
110120
);
Collapse file

‎src/node_dir.cc‎

Copy file name to clipboardExpand all lines: src/node_dir.cc
+18-8Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ using v8::Isolate;
3636
using v8::Local;
3737
using v8::MaybeLocal;
3838
using v8::Null;
39+
using v8::Number;
3940
using v8::Object;
4041
using v8::ObjectTemplate;
4142
using v8::String;
@@ -59,8 +60,8 @@ DirHandle::DirHandle(Environment* env, Local<Object> obj, uv_dir_t* dir)
5960
dir_(dir) {
6061
MakeWeak();
6162

62-
dir_->nentries = arraysize(dirents_);
63-
dir_->dirents = dirents_;
63+
dir_->nentries = 0;
64+
dir_->dirents = nullptr;
6465
}
6566

6667
DirHandle* DirHandle::New(Environment* env, uv_dir_t* dir) {
@@ -230,22 +231,31 @@ void DirHandle::Read(const FunctionCallbackInfo<Value>& args) {
230231
Isolate* isolate = env->isolate();
231232

232233
const int argc = args.Length();
233-
CHECK_GE(argc, 2);
234+
CHECK_GE(argc, 3);
234235

235236
const enum encoding encoding = ParseEncoding(isolate, args[0], UTF8);
236237

237238
DirHandle* dir;
238239
ASSIGN_OR_RETURN_UNWRAP(&dir, args.Holder());
239240

240-
FSReqBase* req_wrap_async = GetReqWrap(env, args[1]);
241-
if (req_wrap_async != nullptr) { // dir.read(encoding, req)
241+
CHECK(args[1]->IsNumber());
242+
uint64_t buffer_size = args[1].As<Number>()->Value();
243+
244+
if (buffer_size != dir->dirents_.size()) {
245+
dir->dirents_.resize(buffer_size);
246+
dir->dir_->nentries = buffer_size;
247+
dir->dir_->dirents = dir->dirents_.data();
248+
}
249+
250+
FSReqBase* req_wrap_async = GetReqWrap(env, args[2]);
251+
if (req_wrap_async != nullptr) { // dir.read(encoding, bufferSize, req)
242252
AsyncCall(env, req_wrap_async, args, "readdir", encoding,
243253
AfterDirRead, uv_fs_readdir, dir->dir());
244-
} else { // dir.read(encoding, undefined, ctx)
245-
CHECK_EQ(argc, 3);
254+
} else { // dir.read(encoding, bufferSize, undefined, ctx)
255+
CHECK_EQ(argc, 4);
246256
FSReqWrapSync req_wrap_sync;
247257
FS_DIR_SYNC_TRACE_BEGIN(readdir);
248-
int err = SyncCall(env, args[2], &req_wrap_sync, "readdir", uv_fs_readdir,
258+
int err = SyncCall(env, args[3], &req_wrap_sync, "readdir", uv_fs_readdir,
249259
dir->dir());
250260
FS_DIR_SYNC_TRACE_END(readdir);
251261
if (err < 0) {
Collapse file

‎src/node_dir.h‎

Copy file name to clipboardExpand all lines: src/node_dir.h
+2-2Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,8 @@ class DirHandle : public AsyncWrap {
4545
void GCClose();
4646

4747
uv_dir_t* dir_;
48-
// Up to 32 directory entries are read through a single libuv call.
49-
uv_dirent_t dirents_[32];
48+
// Multiple entries are read through a single libuv call.
49+
std::vector<uv_dirent_t> dirents_;
5050
bool closing_ = false;
5151
bool closed_ = false;
5252
};
Collapse file

‎test/benchmark/test-benchmark-fs.js‎

Copy file name to clipboardExpand all lines: test/benchmark/test-benchmark-fs.js
+1Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ const tmpdir = require('../common/tmpdir');
77
tmpdir.refresh();
88

99
runBenchmark('fs', [
10+
'bufferSize=32',
1011
'concurrent=1',
1112
'dir=.github',
1213
'dur=0.1',
Collapse file

‎test/parallel/test-fs-opendir.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-fs-opendir.js
+23Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,3 +186,26 @@ async function doAsyncIterThrowTest() {
186186
await assert.rejects(async () => dir.read(), dirclosedError);
187187
}
188188
doAsyncIterThrowTest().then(common.mustCall());
189+
190+
// Check error thrown on invalid values of bufferSize
191+
for (const bufferSize of [-1, 0, 0.5, 1.5, Infinity, NaN]) {
192+
assert.throws(
193+
() => fs.opendirSync(testDir, { bufferSize }),
194+
{
195+
code: 'ERR_OUT_OF_RANGE'
196+
});
197+
}
198+
for (const bufferSize of ['', '1', null]) {
199+
assert.throws(
200+
() => fs.opendirSync(testDir, { bufferSize }),
201+
{
202+
code: 'ERR_INVALID_ARG_TYPE'
203+
});
204+
}
205+
206+
// Check that passing a positive integer as bufferSize works
207+
{
208+
const dir = fs.opendirSync(testDir, { bufferSize: 1024 });
209+
assertDirent(dir.readSync());
210+
dir.close();
211+
}

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.