Skip to content

worker: add worker.getHeapStatistics() - #57888

Merged
nodejs-github-bot merged 19 commits into
nodejs:mainfrom
mcollina:worker-heap-statistics
Apr 17, 2025
Merged

worker: add worker.getHeapStatistics()#57888
nodejs-github-bot merged 19 commits into
nodejs:mainfrom
mcollina:worker-heap-statistics

Conversation

@mcollina

Copy link
Copy Markdown
Member

Adds worker.getHeapStatistics() so that the heap usage of the worker could be observer from the parent thread.

@nodejs-github-botnodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Apr 15, 2025
@mcollina

Copy link
Copy Markdown
MemberAuthor

In node_v8.cc, the same is exposed via

node/src/node_v8.cc

Lines 57 to 71 in 2204587

#defineHEAP_STATISTICS_PROPERTIES(V) \
V(0, total_heap_size, kTotalHeapSizeIndex) \
V(1, total_heap_size_executable, kTotalHeapSizeExecutableIndex) \
V(2, total_physical_size, kTotalPhysicalSizeIndex) \
V(3, total_available_size, kTotalAvailableSize) \
V(4, used_heap_size, kUsedHeapSizeIndex) \
V(5, heap_size_limit, kHeapSizeLimitIndex) \
V(6, malloced_memory, kMallocedMemoryIndex) \
V(7, peak_malloced_memory, kPeakMallocedMemoryIndex) \
V(8, does_zap_garbage, kDoesZapGarbageIndex) \
V(9, number_of_native_contexts, kNumberOfNativeContextsIndex) \
V(10, number_of_detached_contexts, kNumberOfDetachedContextsIndex) \
V(11, total_global_handles_size, kTotalGlobalHandlesSizeIndex) \
V(12, used_global_handles_size, kUsedGlobalHandlesSizeIndex) \
V(13, external_memory, kExternalMemoryIndex)
. Do you want me to use that approach here?

@mcollinamcollina added the semver-minor PRs that contain new features and should be released in the next minor version. label Apr 15, 2025
@mcollina
mcollinaforce-pushed the worker-heap-statistics branch from daafec6 to 85d6960CompareApril 15, 2025 10:36
Adds worker.getHeapStatistics() so that the heap usage of the worker
could be observer from the parent thread.
Signed-off-by: Matteo Collina <hello@matteocollina.com>
Signed-off-by: Matteo Collina <hello@matteocollina.com>
Signed-off-by: Matteo Collina <hello@matteocollina.com>
@mcollina
mcollinaforce-pushed the worker-heap-statistics branch from c10f5e8 to c73d1b6CompareApril 15, 2025 10:59
Comment threadsrc/node_worker.cc Outdated

v8::HeapStatistics heap_stats;
w->isolate_->GetHeapStatistics(&heap_stats);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you use the object constructor that takes an array of keys and array of values, it will be faster

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you got an example I can copy from?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Src/node_v8.cc line 350

Signed-off-by: Matteo Collina <hello@matteocollina.com>
Signed-off-by: Matteo Collina <hello@matteocollina.com>
Signed-off-by: Matteo Collina <hello@matteocollina.com>

@QardQard left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only thing that might be of concern would be that placing this on the worker object loses the isolation as v8-specific that the one in the v8 module has. An alternative implementation could be something like v8.getWorkerHeapStatistics(worker) to keep it in that v8-specific segment of the API, if that's something we care about. I would not block on this, but I figured it worth bringing up.

Other than that, agreed that the array form object construction would be better.

@codecov

codecovBot commented Apr 15, 2025

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 90.21739% with 9 lines in your changes missing coverage. Please review.

Project coverage is 90.23%. Comparing base (2204587) to head (df57fa3).
Report is 15 commits behind head on main.

Files with missing linesPatch %Lines
src/node_worker.cc88.88%5 Missing and 4 partials ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #57888 +/- ##
==========================================
- Coverage 90.24% 90.23% -0.01% 
==========================================
Files 630 630 Lines 185688 185783 +95 Branches 36403 36414 +11 ==========================================
+ Hits 167569 167647 +78 - Misses 10997 11009 +12 - Partials 7122 7127 +5 
Files with missing linesCoverage Δ
lib/internal/worker.js99.82% <100.00%> (+<0.01%)⬆️
src/node_worker.h90.90% <ø> (ø)
src/node_worker.cc84.45% <88.88%> (+0.72%)⬆️

... and 27 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment threadsrc/node_worker.cc Outdated
@legendecas

Copy link
Copy Markdown
Member

Only thing that might be of concern would be that placing this on the worker object loses the isolation as v8-specific that the one in the v8 module has.

There is a precedent of Worker.getHeapSnapshot, coresponding to v8.getHeapSnapshot.

@mcollina

Copy link
Copy Markdown
MemberAuthor

Only thing that might be of concern would be that placing this on the worker object loses the isolation as v8-specific that the one in the v8 module has.

There is also the event loop utilization monitoring that can be done from the outside, too.

Comment threadsrc/node_worker.cc Outdated
Signed-off-by: Matteo Collina <hello@matteocollina.com>
Signed-off-by: Matteo Collina <hello@matteocollina.com>
Signed-off-by: Matteo Collina <hello@matteocollina.com>
@mcollina

Copy link
Copy Markdown
MemberAuthor

@joyeecheung@legendecas PTAL.

Signed-off-by: Matteo Collina <hello@matteocollina.com>
Signed-off-by: Matteo Collina <hello@matteocollina.com>
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 16, 2025
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@mcollinamcollina added the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 17, 2025
@RaisinTenRaisinTen added the commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. label Apr 17, 2025
@nodejs-github-botnodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 17, 2025
@nodejs-github-bot
nodejs-github-bot merged commit 33d8e03 into nodejs:mainApr 17, 2025
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Landed in 33d8e03

@mcollina
mcollina deleted the worker-heap-statistics branch April 17, 2025 13:43
RafaelGSS pushed a commit that referenced this pull request May 1, 2025
Adds worker.getHeapStatistics() so that the heap usage of the worker
could be observer from the parent thread.
Signed-off-by: Matteo Collina <hello@matteocollina.com>
PR-URL: #57888
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
RafaelGSS pushed a commit that referenced this pull request May 2, 2025
Adds worker.getHeapStatistics() so that the heap usage of the worker
could be observer from the parent thread.
Signed-off-by: Matteo Collina <hello@matteocollina.com>
PR-URL: #57888
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
aduh95 pushed a commit that referenced this pull request May 6, 2025
Adds worker.getHeapStatistics() so that the heap usage of the worker
could be observer from the parent thread.
Signed-off-by: Matteo Collina <hello@matteocollina.com>
PR-URL: #57888
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
aduh95 pushed a commit that referenced this pull request May 6, 2025
Adds worker.getHeapStatistics() so that the heap usage of the worker
could be observer from the parent thread.
Signed-off-by: Matteo Collina <hello@matteocollina.com>
PR-URL: #57888
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
RafaelGSS pushed a commit that referenced this pull request May 14, 2025
Adds worker.getHeapStatistics() so that the heap usage of the worker
could be observer from the parent thread.
Signed-off-by: Matteo Collina <hello@matteocollina.com>
PR-URL: #57888
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
aduh95 pushed a commit that referenced this pull request May 16, 2025
Adds worker.getHeapStatistics() so that the heap usage of the worker
could be observer from the parent thread.
Signed-off-by: Matteo Collina <hello@matteocollina.com>
PR-URL: #57888
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
aduh95 pushed a commit that referenced this pull request May 17, 2025
Adds worker.getHeapStatistics() so that the heap usage of the worker
could be observer from the parent thread.
Signed-off-by: Matteo Collina <hello@matteocollina.com>
PR-URL: #57888
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
aduh95 pushed a commit that referenced this pull request May 18, 2025
Adds worker.getHeapStatistics() so that the heap usage of the worker
could be observer from the parent thread.
Signed-off-by: Matteo Collina <hello@matteocollina.com>
PR-URL: #57888
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
aduh95 pushed a commit that referenced this pull request May 19, 2025
Adds worker.getHeapStatistics() so that the heap usage of the worker
could be observer from the parent thread.
Signed-off-by: Matteo Collina <hello@matteocollina.com>
PR-URL: #57888
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
aduh95 added a commit that referenced this pull request May 19, 2025
Notable changes:
deps:
* update timezone to 2025b (Node.js GitHub Bot) #57857
doc:
* add dario-piotrowicz to collaborators (Dario Piotrowicz) #58102
* (SEMVER-MINOR) graduate multiple experimental apis (James M Snell) #57765
esm:
* (SEMVER-MINOR) graduate import.meta properties (James M Snell) #58011
* (SEMVER-MINOR) support top-level Wasm without package type (Guy Bedford) #57610
sqlite:
* (SEMVER-MINOR) add StatementSync.prototype.columns() (Colin Ihrig) #57490
src:
* (SEMVER-MINOR) set default config as `node.config.json` (Marco Ippolito) #57171
* (SEMVER-MINOR) create `THROW_ERR_OPTIONS_BEFORE_BOOTSTRAPPING` (Marco Ippolito) #57016
* (SEMVER-MINOR) add config file support (Marco Ippolito) #57016
* (SEMVER-MINOR) add ExecutionAsyncId getter for any Context (Attila Szegedi) #57820
stream:
* (SEMVER-MINOR) preserve AsyncLocalStorage context in finished() (Gürgün Dayıoğlu) #57865
util:
* (SEMVER-MINOR) add `types.isFloat16Array()` (Livia Medeiros) #57879
worker:
* (SEMVER-MINOR) add worker.getHeapStatistics() (Matteo Collina) #57888
PR-URL: #58388
aduh95 added a commit that referenced this pull request May 21, 2025
Notable changes:
deps:
* update timezone to 2025b (Node.js GitHub Bot) #57857
doc:
* add dario-piotrowicz to collaborators (Dario Piotrowicz) #58102
* (SEMVER-MINOR) graduate multiple experimental apis (James M Snell) #57765
esm:
* (SEMVER-MINOR) graduate import.meta properties (James M Snell) #58011
* (SEMVER-MINOR) support top-level Wasm without package type (Guy Bedford) #57610
sqlite:
* (SEMVER-MINOR) add StatementSync.prototype.columns() (Colin Ihrig) #57490
src:
* (SEMVER-MINOR) set default config as `node.config.json` (Marco Ippolito) #57171
* (SEMVER-MINOR) create `THROW_ERR_OPTIONS_BEFORE_BOOTSTRAPPING` (Marco Ippolito) #57016
* (SEMVER-MINOR) add config file support (Marco Ippolito) #57016
* (SEMVER-MINOR) add ExecutionAsyncId getter for any Context (Attila Szegedi) #57820
stream:
* (SEMVER-MINOR) preserve AsyncLocalStorage context in finished() (Gürgün Dayıoğlu) #57865
util:
* (SEMVER-MINOR) add `types.isFloat16Array()` (Livia Medeiros) #57879
worker:
* (SEMVER-MINOR) add worker.getHeapStatistics() (Matteo Collina) #57888
PR-URL: #58388
@ghostghost mentioned this pull request Jun 8, 2025
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++Issues and PRs that require attention from people who are familiar with C++.commit-queue-squashAdd this label to instruct the Commit Queue to squash all the PR commits into the first one.lib / srcIssues and PRs related to general changes in the lib or src directory.needs-ciPRs that need a full CI run.semver-minorPRs that contain new features and should be released in the next minor version.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants

@mcollina@legendecas@nodejs-github-bot@Qard@anonrig@joyeecheung@RaisinTen