Skip to content

Conversation

@caiolima
Copy link
Contributor

As noted by @targos in #60429 (comment), the implementation of total_allocated_bytes changes the ABI. This PR changes original V8 implementation to a version where there's no addition of HeapStatistics::total_allocated_bytes_ to V8's HeapStatistics, and instead we get such values using Isolate::GetTotalAllocatedBytes. This also changes code introduced by #60573 to use this new API on either v8.getHeapStatistics() and Worker::GetHeapStatistics. The goal here is to make total_allocated_bytes back portable to previous versions, but keeping the v8.getHeapStatistics() API stable once users transition from v25 to future versions. It also solves the ABI change introduced by https://github.com/nodejs/node/pull/60429/files

There's also a fix on documentation text with missing . and updating the code snippet to include total_allocated_bytes on output.

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/gyp
  • @nodejs/security-wg
  • @nodejs/v8-update

@nodejs-github-bot nodejs-github-bot added lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. v25.x Issues that can be reproduced on v25.x or PRs targeting the v25.x-staging branch. labels Nov 21, 2025
Copy link
Member

@RafaelGSS RafaelGSS left a comment

Choose a reason for hiding this comment

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

LGTM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. v25.x Issues that can be reproduced on v25.x or PRs targeting the v25.x-staging branch.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants