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 216e200

Browse filesBrowse files
addaleaxtargos
addaleax
authored and
targos
committed
fs: buffer dir entries in opendir()
Read up to 32 directory entries in one batch when `dir.readSync()` or `dir.read()` are called. This increases performance significantly, although it introduces quite a bit of edge case complexity. confidence improvement accuracy (*) (**) (***) fs/bench-opendir.js mode='async' dir='lib' n=100 *** 155.93 % ±30.05% ±40.34% ±53.21% fs/bench-opendir.js mode='async' dir='test/parallel' n=100 *** 479.65 % ±56.81% ±76.47% ±101.32% fs/bench-opendir.js mode='sync' dir='lib' n=100 10.38 % ±14.39% ±19.16% ±24.96% fs/bench-opendir.js mode='sync' dir='test/parallel' n=100 *** 63.13 % ±12.84% ±17.18% ±22.58% PR-URL: #29893 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: David Carlier <devnexen@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
1 parent e16e3d5 commit 216e200
Copy full SHA for 216e200

File tree

Expand file treeCollapse file tree

5 files changed

+143
-45
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

5 files changed

+143
-45
lines changed
Open diff view settings
Collapse file

‎benchmark/fs/bench-opendir.js‎

Copy file name to clipboard
+51Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
const fs = require('fs');
5+
const path = require('path');
6+
7+
const bench = common.createBenchmark(main, {
8+
n: [100],
9+
dir: [ 'lib', 'test/parallel'],
10+
mode: [ 'async', 'sync', 'callback' ]
11+
});
12+
13+
async function main({ n, dir, mode }) {
14+
const fullPath = path.resolve(__dirname, '../../', dir);
15+
16+
bench.start();
17+
18+
let counter = 0;
19+
for (let i = 0; i < n; i++) {
20+
if (mode === 'async') {
21+
// eslint-disable-next-line no-unused-vars
22+
for await (const entry of await fs.promises.opendir(fullPath))
23+
counter++;
24+
} else if (mode === 'callback') {
25+
const dir = await fs.promises.opendir(fullPath);
26+
await new Promise((resolve, reject) => {
27+
function read() {
28+
dir.read((err, entry) => {
29+
if (err) {
30+
reject(err);
31+
} else if (entry === null) {
32+
resolve(dir.close());
33+
} else {
34+
counter++;
35+
read();
36+
}
37+
});
38+
}
39+
40+
read();
41+
});
42+
} else {
43+
const dir = fs.opendirSync(fullPath);
44+
while (dir.readSync() !== null)
45+
counter++;
46+
dir.closeSync();
47+
}
48+
}
49+
50+
bench.end(counter);
51+
}
Collapse file

‎lib/internal/fs/dir.js‎

Copy file name to clipboardExpand all lines: lib/internal/fs/dir.js
+26-1Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,23 +24,27 @@ const {
2424

2525
const kDirHandle = Symbol('kDirHandle');
2626
const kDirPath = Symbol('kDirPath');
27+
const kDirBufferedEntries = Symbol('kDirBufferedEntries');
2728
const kDirClosed = Symbol('kDirClosed');
2829
const kDirOptions = Symbol('kDirOptions');
30+
const kDirReadImpl = Symbol('kDirReadImpl');
2931
const kDirReadPromisified = Symbol('kDirReadPromisified');
3032
const kDirClosePromisified = Symbol('kDirClosePromisified');
3133

3234
class Dir {
3335
constructor(handle, path, options) {
3436
if (handle == null) throw new ERR_MISSING_ARGS('handle');
3537
this[kDirHandle] = handle;
38+
this[kDirBufferedEntries] = [];
3639
this[kDirPath] = path;
3740
this[kDirClosed] = false;
3841

3942
this[kDirOptions] = getOptions(options, {
4043
encoding: 'utf8'
4144
});
4245

43-
this[kDirReadPromisified] = internalUtil.promisify(this.read).bind(this);
46+
this[kDirReadPromisified] =
47+
internalUtil.promisify(this[kDirReadImpl]).bind(this, false);
4448
this[kDirClosePromisified] = internalUtil.promisify(this.close).bind(this);
4549
}
4650

@@ -49,6 +53,10 @@ class Dir {
4953
}
5054

5155
read(callback) {
56+
return this[kDirReadImpl](true, callback);
57+
}
58+
59+
[kDirReadImpl](maybeSync, callback) {
5260
if (this[kDirClosed] === true) {
5361
throw new ERR_DIR_CLOSED();
5462
}
@@ -59,11 +67,22 @@ class Dir {
5967
throw new ERR_INVALID_CALLBACK(callback);
6068
}
6169

70+
if (this[kDirBufferedEntries].length > 0) {
71+
const [ name, type ] = this[kDirBufferedEntries].splice(0, 2);
72+
if (maybeSync)
73+
process.nextTick(getDirent, this[kDirPath], name, type, callback);
74+
else
75+
getDirent(this[kDirPath], name, type, callback);
76+
return;
77+
}
78+
6279
const req = new FSReqCallback();
6380
req.oncomplete = (err, result) => {
6481
if (err || result === null) {
6582
return callback(err, result);
6683
}
84+
85+
this[kDirBufferedEntries] = result.slice(2);
6786
getDirent(this[kDirPath], result[0], result[1], callback);
6887
};
6988

@@ -78,6 +97,11 @@ class Dir {
7897
throw new ERR_DIR_CLOSED();
7998
}
8099

100+
if (this[kDirBufferedEntries].length > 0) {
101+
const [ name, type ] = this[kDirBufferedEntries].splice(0, 2);
102+
return getDirent(this[kDirPath], name, type);
103+
}
104+
81105
const ctx = { path: this[kDirPath] };
82106
const result = this[kDirHandle].read(
83107
this[kDirOptions].encoding,
@@ -90,6 +114,7 @@ class Dir {
90114
return result;
91115
}
92116

117+
this[kDirBufferedEntries] = result.slice(2);
93118
return getDirent(this[kDirPath], result[0], result[1]);
94119
}
95120

Collapse file

‎src/node_dir.cc‎

Copy file name to clipboardExpand all lines: src/node_dir.cc
+52-40Lines changed: 52 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -59,8 +59,8 @@ DirHandle::DirHandle(Environment* env, Local<Object> obj, uv_dir_t* dir)
5959
dir_(dir) {
6060
MakeWeak();
6161

62-
dir_->nentries = 1;
63-
dir_->dirents = &dirent_;
62+
dir_->nentries = arraysize(dirents_);
63+
dir_->dirents = dirents_;
6464
}
6565

6666
DirHandle* DirHandle::New(Environment* env, uv_dir_t* dir) {
@@ -160,7 +160,37 @@ void DirHandle::Close(const FunctionCallbackInfo<Value>& args) {
160160
}
161161
}
162162

163-
void AfterDirReadSingle(uv_fs_t* req) {
163+
static MaybeLocal<Array> DirentListToArray(
164+
Environment* env,
165+
uv_dirent_t* ents,
166+
int num,
167+
enum encoding encoding,
168+
Local<Value>* err_out) {
169+
MaybeStackBuffer<Local<Value>, 96> entries(num * 3);
170+
171+
// Return an array of all read filenames.
172+
int j = 0;
173+
for (int i = 0; i < num; i++) {
174+
Local<Value> filename;
175+
Local<Value> error;
176+
const size_t namelen = strlen(ents[i].name);
177+
if (!StringBytes::Encode(env->isolate(),
178+
ents[i].name,
179+
namelen,
180+
encoding,
181+
&error).ToLocal(&filename)) {
182+
*err_out = error;
183+
return MaybeLocal<Array>();
184+
}
185+
186+
entries[j++] = filename;
187+
entries[j++] = Integer::New(env->isolate(), ents[i].type);
188+
}
189+
190+
return Array::New(env->isolate(), entries.out(), j);
191+
}
192+
193+
static void AfterDirRead(uv_fs_t* req) {
164194
FSReqBase* req_wrap = FSReqBase::from_req(req);
165195
FSReqAfterScope after(req_wrap, req);
166196

@@ -170,7 +200,6 @@ void AfterDirReadSingle(uv_fs_t* req) {
170200

171201
Environment* env = req_wrap->env();
172202
Isolate* isolate = env->isolate();
173-
Local<Value> error;
174203

175204
if (req->result == 0) {
176205
// Done
@@ -182,26 +211,17 @@ void AfterDirReadSingle(uv_fs_t* req) {
182211
uv_dir_t* dir = static_cast<uv_dir_t*>(req->ptr);
183212
req->ptr = nullptr;
184213

185-
// Single entries are returned without an array wrapper
186-
const uv_dirent_t& ent = dir->dirents[0];
187-
188-
MaybeLocal<Value> filename =
189-
StringBytes::Encode(isolate,
190-
ent.name,
191-
req_wrap->encoding(),
192-
&error);
193-
if (filename.IsEmpty())
214+
Local<Value> error;
215+
Local<Array> js_array;
216+
if (!DirentListToArray(env,
217+
dir->dirents,
218+
req->result,
219+
req_wrap->encoding(),
220+
&error).ToLocal(&js_array)) {
194221
return req_wrap->Reject(error);
222+
}
195223

196-
197-
Local<Array> result = Array::New(isolate, 2);
198-
result->Set(env->context(),
199-
0,
200-
filename.ToLocalChecked()).FromJust();
201-
result->Set(env->context(),
202-
1,
203-
Integer::New(isolate, ent.type)).FromJust();
204-
req_wrap->Resolve(result);
224+
req_wrap->Resolve(js_array);
205225
}
206226

207227

@@ -217,10 +237,10 @@ void DirHandle::Read(const FunctionCallbackInfo<Value>& args) {
217237
DirHandle* dir;
218238
ASSIGN_OR_RETURN_UNWRAP(&dir, args.Holder());
219239

220-
FSReqBase* req_wrap_async = static_cast<FSReqBase*>(GetReqWrap(env, args[1]));
240+
FSReqBase* req_wrap_async = GetReqWrap(env, args[1]);
221241
if (req_wrap_async != nullptr) { // dir.read(encoding, req)
222242
AsyncCall(env, req_wrap_async, args, "readdir", encoding,
223-
AfterDirReadSingle, uv_fs_readdir, dir->dir());
243+
AfterDirRead, uv_fs_readdir, dir->dir());
224244
} else { // dir.read(encoding, undefined, ctx)
225245
CHECK_EQ(argc, 3);
226246
FSReqWrapSync req_wrap_sync;
@@ -240,28 +260,20 @@ void DirHandle::Read(const FunctionCallbackInfo<Value>& args) {
240260
}
241261

242262
CHECK_GE(req_wrap_sync.req.result, 0);
243-
const uv_dirent_t& ent = dir->dir()->dirents[0];
244263

245264
Local<Value> error;
246-
MaybeLocal<Value> filename =
247-
StringBytes::Encode(isolate,
248-
ent.name,
249-
encoding,
250-
&error);
251-
if (filename.IsEmpty()) {
265+
Local<Array> js_array;
266+
if (!DirentListToArray(env,
267+
dir->dir()->dirents,
268+
req_wrap_sync.req.result,
269+
encoding,
270+
&error).ToLocal(&js_array)) {
252271
Local<Object> ctx = args[2].As<Object>();
253-
ctx->Set(env->context(), env->error_string(), error).FromJust();
272+
USE(ctx->Set(env->context(), env->error_string(), error));
254273
return;
255274
}
256275

257-
Local<Array> result = Array::New(isolate, 2);
258-
result->Set(env->context(),
259-
0,
260-
filename.ToLocalChecked()).FromJust();
261-
result->Set(env->context(),
262-
1,
263-
Integer::New(isolate, ent.type)).FromJust();
264-
args.GetReturnValue().Set(result);
276+
args.GetReturnValue().Set(js_array);
265277
}
266278
}
267279

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
@@ -25,7 +25,6 @@ class DirHandle : public AsyncWrap {
2525
static void Close(const v8::FunctionCallbackInfo<Value>& args);
2626

2727
inline uv_dir_t* dir() { return dir_; }
28-
AsyncWrap* GetAsyncWrap() { return this; }
2928

3029
void MemoryInfo(MemoryTracker* tracker) const override {
3130
tracker->TrackFieldWithSize("dir", sizeof(*dir_));
@@ -46,7 +45,8 @@ class DirHandle : public AsyncWrap {
4645
void GCClose();
4746

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

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

Copy file name to clipboardExpand all lines: test/parallel/test-fs-opendir.js
+12-2Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,17 +58,27 @@ const dirclosedError = {
5858
// Check the opendir async version
5959
fs.opendir(testDir, common.mustCall(function(err, dir) {
6060
assert.ifError(err);
61-
dir.read(common.mustCall(function(err, dirent) {
61+
let sync = true;
62+
dir.read(common.mustCall((err, dirent) => {
63+
assert(!sync);
6264
assert.ifError(err);
6365

6466
// Order is operating / file system dependent
6567
assert(files.includes(dirent.name), `'files' should include ${dirent}`);
6668
assertDirent(dirent);
6769

68-
dir.close(common.mustCall(function(err) {
70+
let syncInner = true;
71+
dir.read(common.mustCall((err, dirent) => {
72+
assert(!syncInner);
6973
assert.ifError(err);
74+
75+
dir.close(common.mustCall(function(err) {
76+
assert.ifError(err);
77+
}));
7078
}));
79+
syncInner = false;
7180
}));
81+
sync = false;
7282
}));
7383

7484
// opendir() on file should throw ENOTDIR

0 commit comments

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