FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fs: add `bufferSize` option to `fs.opendir()` · nodejs/node@07b5584 · GitHub

/ node Public

Commit 07b5584

Browse files
addaleax
authored and
targos
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 e4ab6fc commit 07b5584

7 files changed

Lines changed: 85 additions & 18 deletions

File tree

‎benchmark/fs/bench-opendir.js‎

Lines 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();

‎doc/api/fs.md‎

Lines changed: 21 additions & 0 deletions
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).

‎lib/internal/fs/dir.js‎

Lines 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
);

‎src/node_dir.cc‎

Lines 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) {

‎src/node_dir.h‎

Lines 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
};

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

Lines 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',

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

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

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL