Skip to content

[v24.x backport] v8: adding total_allocated_bytes to HeapStatistics - #60801

Draft
caiolima wants to merge 6 commits into
nodejs:v24.x-stagingfrom
caiolima:brackport-60573-to-v24
Draft

[v24.x backport] v8: adding total_allocated_bytes to HeapStatistics#60801
caiolima wants to merge 6 commits into
nodejs:v24.x-stagingfrom
caiolima:brackport-60573-to-v24

Conversation

@caiolima

@caiolimacaiolima commented Nov 21, 2025

Copy link
Copy Markdown
Contributor

This PR contains backport of #60573 to v24. Since it depends on #60429, it also includes a backport version of this PR as well.

To avoid ABI changes, we are also taking in consideration the changes proposed on #60800 from original V8 PR where Isolate:: GetTotalAllocatedBytes is introduced as an alternative method to get total_allocated_bytes_. This avoids ABI breakage due to changes made on HeapStatistics.

caiolimaand others added 2 commits November 21, 2025 07:28
Original commit message:
[api] Adding total allocated bytes in HeapStatistics
This change exposes total allocated bytes in v8::HeapStatistics API by
introducing a new total_allocated_bytes() method that tracks all heap
allocations since an Isolate creation.
The implementation adds:
- uint64_t total_allocated_bytes_ field to HeapStatistics.
- An atomic total allocation counter is stored in the Heap class.
- The counter is incremented whenever a RestLab is called. This approach can overestimate the total allocation for cases where the LAB is not fully used, but the leftover compared to the LAB itself is quite small, so it seems tolerable.
Design doc reference:
https://docs.google.com/document/d/1O4JPsoaxTQsX_7T5Fz4rsGeHMiM16jUrvDuq9FrtbNM
Change-Id: Ic531698aaeb1578f943b7fdd346b9159ffd9b6c9
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6996467
Reviewed-by: Dominik Inführ <dinfuehr@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Commit-Queue: Dmitry Bezhetskov <dima00782@gmail.com>
Cr-Commit-Position: refs/heads/main@{#103296}
Refs: v8/v8@fe81545
Co-authored-by: Caio Lima <caiolima@igalia.com>
PR-URL: nodejs#60429
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
PR-URL: nodejs#60573
Reviewed-By: theanarkh <theratliter@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
fixing tab error.
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

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

@nodejs-github-botnodejs-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. v24.x Issues that can be reproduced on v24.x or PRs targeting the v24.x-staging branch. labels Nov 21, 2025
@richardlau

Copy link
Copy Markdown
Member

To avoid ABI changes, we are also taking in consideration the changes proposed on #60800 from original V8 PR where Isolate:: GetTotalAllocatedBytes is introduced as an alternative method to get total_allocated_bytes_. This avoids ABI breakage due to changes made on HeapStatistics.

I'm no expert on ABI's, but doesn't that change just shift the breakage to Isolate?

@caiolima

Copy link
Copy Markdown
ContributorAuthor

I'm no expert on ABI's, but doesn't that change just shift the breakage to Isolate?

I'm not an expert either, so my understanding might be faulty. IIUC, the addition of new non-virtual methods doesn't break ABI, while changing the layout of HeapStatistics do. Any program that compiles with previous include/v8-isolate.h would have no issues compiling with this new header. However with HeapStatistics layout change there will be a mismatch of structures and it will be problematic.

@caiolima

Copy link
Copy Markdown
ContributorAuthor

I'm also considering here that the addition of total_allocated_bytes_ on Heap is fine, since it's not exposed outside V8.

luyahanand others added 4 commits December 10, 2025 17:18
Original commit message:
Reland "[riscv] Fix Check failed in bind_to"
This is a reland of commit fdb5de2c741658e94944f2ec1218530e98601c23
Original change's description:
> [riscv] Fix Check failed in bind_to
>
> The trampoline should be emitted before the constant pool.
>
> Bug: 420232092
>
> Change-Id: I3a909b122607e37aca9d8765f28810ec74d5dc0b
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6578135
> Auto-Submit: Yahan Lu (LuYahan) <yahan@iscas.ac.cn>
> Reviewed-by: Ji Qiu <qiuji@iscas.ac.cn>
> Commit-Queue: Ji Qiu <qiuji@iscas.ac.cn>
> Cr-Commit-Position: refs/heads/main@{#100480}
Bug: 420232092
Change-Id: I1fac1ed8c349383ef4510abea338b3d695ed57ab
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6595668
Commit-Queue: Ji Qiu <qiuji@iscas.ac.cn>
Reviewed-by: Ji Qiu <qiuji@iscas.ac.cn>
Cr-Commit-Position: refs/heads/main@{#100745}
Refs: v8/v8@bbaae8e
Co-authored-by: kxxt <rsworktech@outlook.com>
PR-URL: nodejs#60962
Reviewed-By: Richard Lau <richard.lau@ibm.com>
Reviewed-By: Stewart X Addison <sxa@redhat.com>
Original commit message:
[riscv] Check trampoline before Constant pool in Release mode
Change-Id: I9645cded9328dabb2c11c7859b998c838b95f97b
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6653368
Commit-Queue: Ji Qiu <qiuji@iscas.ac.cn>
Reviewed-by: Ji Qiu <qiuji@iscas.ac.cn>
Cr-Commit-Position: refs/heads/main@{#100935}
Refs: v8/v8@394a805
PR-URL: nodejs#60962
Reviewed-By: Richard Lau <richard.lau@ibm.com>
Reviewed-By: Stewart X Addison <sxa@redhat.com>
Original commit message:
[riscv] Fix the RISC-V build.
Due to recent changes, there were missing implementations of various
methods needed in the simulator and Maglev. Additionally, a static_cast
is needed in the assembler to silence a warning.
Port commit dfc894cd22d86ce42830e3bfdf485d963f6396ad
Port commit c33af9bd408eadd6b62571f862bcb5b763c98ad9
Change-Id: Ie37a1cfa8225fc12f367ff62139cc7cd8fa967d0
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6396542
Reviewed-by: Victor Gomes <victorgomes@chromium.org>
Commit-Queue: Yahan Lu (LuYahan) <yahan@iscas.ac.cn>
Reviewed-by: Ji Qiu <qiuji@iscas.ac.cn>
Reviewed-by: Yahan Lu (LuYahan) <yahan@iscas.ac.cn>
Cr-Commit-Position: refs/heads/main@{#99706}
Refs: v8/v8@1441665
PR-URL: nodejs#60989
Reviewed-By: Richard Lau <richard.lau@ibm.com>
Reviewed-By: Beth Griggs <bethanyngriggs@gmail.com>
@aduh95
aduh95force-pushed the v24.x-staging branch 2 times, most recently from 065c9b0 to 625b90bCompareFebruary 10, 2026 13:17
@aduh95
aduh95 marked this pull request as draft April 3, 2026 09:22
@aduh95
aduh95force-pushed the v24.x-staging branch 5 times, most recently from cfd843d to a072411CompareApril 10, 2026 14:18
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@caiolima@nodejs-github-bot@richardlau@luyahan@dstolfa