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

fs: allocate FSReqPromise stat arrays lazily · nodejs/node@c5cb6bc · GitHub

/ node Public

Commit c5cb6bc

Browse files
MarshallOfSound
authored and
aduh95
committed
fs: allocate FSReqPromise stat arrays lazily
Every promise-based fs operation eagerly allocated two AliasedBuffers (a stats array and a statfs array) at request creation, although only stat-family resolutions ever read the first and only statfs() reads the second. Each allocation is an ArrayBuffer, a TypedArray and a strong v8::Global. The callback path has no equivalent cost since it resolves through a shared global array. Construct the arrays lazily in ResolveStat()/ResolveStatFs() instead. Once created the lifetime is unchanged, so deferred continuations still read from request-owned memory. Improves fs/promises throughput under concurrency: writeFile +53%, stat +26%, readFile +22% at 64 in-flight operations on tmpfs, with callback paths unchanged. Signed-off-by: Sam Attard <sattard@anthropic.com> PR-URL: #63886 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Edy Silva <edigleyssonsilva@gmail.com>
1 parent b405e9b commit c5cb6bc

3 files changed

Lines changed: 29 additions & 18 deletions

File tree

‎src/node_file-inl.h‎

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -209,13 +209,7 @@ FSReqPromise<AliasedBufferT>::FSReqPromise(BindingData* binding_data,
209209
v8::Local<v8::Object> obj,
210210
bool use_bigint)
211211
: FSReqBase(
212-
binding_data, obj, AsyncWrap::PROVIDER_FSREQPROMISE, use_bigint),
213-
stats_field_array_(
214-
env()->isolate(),
215-
static_cast<size_t>(FsStatsOffset::kFsStatsFieldsNumber)),
216-
statfs_field_array_(
217-
env()->isolate(),
218-
static_cast<size_t>(FsStatFsOffset::kFsStatFsFieldsNumber)) {}
212+
binding_data, obj, AsyncWrap::PROVIDER_FSREQPROMISE, use_bigint) {}
219213

220214
template <typename AliasedBufferT>
221215
void FSReqPromise<AliasedBufferT>::Reject(v8::Local<v8::Value> reject) {
@@ -253,14 +247,24 @@ void FSReqPromise<AliasedBufferT>::Resolve(v8::Local<v8::Value> value) {
253247

254248
template <typename AliasedBufferT>
255249
void FSReqPromise<AliasedBufferT>::ResolveStat(const uv_stat_t* stat) {
256-
FillStatsArray(&stats_field_array_, stat);
257-
Resolve(stats_field_array_.GetJSArray());
250+
if (!stats_field_array_.has_value()) {
251+
stats_field_array_.emplace(
252+
env()->isolate(),
253+
static_cast<size_t>(FsStatsOffset::kFsStatsFieldsNumber));
254+
}
255+
FillStatsArray(&stats_field_array_.value(), stat);
256+
Resolve(stats_field_array_->GetJSArray());
258257
}
259258

260259
template <typename AliasedBufferT>
261260
void FSReqPromise<AliasedBufferT>::ResolveStatFs(const uv_statfs_t* stat) {
262-
FillStatFsArray(&statfs_field_array_, stat);
263-
Resolve(statfs_field_array_.GetJSArray());
261+
if (!statfs_field_array_.has_value()) {
262+
statfs_field_array_.emplace(
263+
env()->isolate(),
264+
static_cast<size_t>(FsStatFsOffset::kFsStatFsFieldsNumber));
265+
}
266+
FillStatFsArray(&statfs_field_array_.value(), stat);
267+
Resolve(statfs_field_array_->GetJSArray());
264268
}
265269

266270
template <typename AliasedBufferT>
@@ -280,8 +284,12 @@ void FSReqPromise<AliasedBufferT>::SetReturnValue(
280284
template <typename AliasedBufferT>
281285
void FSReqPromise<AliasedBufferT>::MemoryInfo(MemoryTracker* tracker) const {
282286
FSReqBase::MemoryInfo(tracker);
283-
tracker->TrackField("stats_field_array", stats_field_array_);
284-
tracker->TrackField("statfs_field_array", statfs_field_array_);
287+
if (stats_field_array_.has_value()) {
288+
tracker->TrackField("stats_field_array", stats_field_array_.value());
289+
}
290+
if (statfs_field_array_.has_value()) {
291+
tracker->TrackField("statfs_field_array", statfs_field_array_.value());
292+
}
285293
}
286294

287295
FSReqBase* GetReqWrap(const v8::FunctionCallbackInfo<v8::Value>& args,

‎src/node_file.h‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -266,8 +266,11 @@ class FSReqPromise final : public FSReqBase {
266266
bool use_bigint);
267267

268268
bool finished_ = false;
269-
AliasedBufferT stats_field_array_;
270-
AliasedBufferT statfs_field_array_;
269+
// Constructed lazily in ResolveStat()/ResolveStatFs(): most operations
270+
// never resolve with stats, and eagerly allocating the backing stores
271+
// for every request is a significant per-request cost.
272+
std::optional<AliasedBufferT> stats_field_array_;
273+
std::optional<AliasedBufferT> statfs_field_array_;
271274
};
272275

273276
class FSReqAfterScope final {

‎test/pummel/test-heapdump-fs-promise.js‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ fs.stat(__filename);
2020
validateByRetainingPathFromNodes(nodes, 'Node / FSReqPromise', [
2121
{ node_name: 'FSReqPromise', edge_name: 'native_to_javascript' },
2222
]);
23-
validateByRetainingPathFromNodes(nodes, 'Node / FSReqPromise', [
24-
{ node_name: 'Node / AliasedFloat64Array', edge_name: 'stats_field_array' },
25-
]);
23+
// The stats field array is allocated lazily when the request resolves
24+
// with stats, so it is not retained by a request that is still pending
25+
// and cannot be observed in a heap snapshot.
2626
}

0 commit comments

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