[heap, profiler] Emit weak edges for WeakMember in cppgc [v8/v8 : main]
0 views
Skip to first unread message
Dominik Inführ (Gerrit)
May 8, 2026, 5:44:44 AMMay 8
to Michael Lippautz, v8-s...@luci-project-accounts.iam.gserviceaccount.com, android-bu...@system.gserviceaccount.com, Hannes Payer, cbruni...@chromium.org, devtools-...@chromium.org, mlippau...@chromium.org, oilpan-r...@chromium.org, v8-re...@googlegroups.com
Attention needed from Michael Lippautz
Dominik Inführ voted and added 1 comment![]( "Open in Gerrit")
Votes added by Dominik Inführ
| Commit-Queue | +1 |
1 comment
Patchset-level comments
Related details
Attention is currently required from:
- Michael Lippautz
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Michael Lippautz (Gerrit)
May 8, 2026, 11:07:38 AMMay 8
to Dominik Inführ, v8-s...@luci-project-accounts.iam.gserviceaccount.com, android-bu...@system.gserviceaccount.com, Hannes Payer, cbruni...@chromium.org, devtools-...@chromium.org, mlippau...@chromium.org, oilpan-r...@chromium.org, v8-re...@googlegroups.com
Attention needed from Dominik Inführ
Michael Lippautz voted and added 2 comments![]( "Open in Gerrit")
Votes added by Michael Lippautz
| Code-Review | +1 |
2 comments
Patchset-level comments
Michael Lippautz . resolved
lgtm w/ question
File src/profiler/heap-snapshot-generator.cc
Line 3466, Patchset 2 (Latest):
from->SetNamedAutoIndexReference(HeapGraphEdge::kWeak, nullptr, to,Michael Lippautz . unresolved
Why `SetNamedAutoIndexReference` vs `SetIndexedAutoIndexReference`?
Related details
Attention is currently required from:
- Dominik Inführ
Submit Requirements:
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Dominik Inführ (Gerrit)
May 11, 2026, 1:34:31 AMMay 11
to Michael Lippautz, v8-s...@luci-project-accounts.iam.gserviceaccount.com, android-bu...@system.gserviceaccount.com, Hannes Payer, cbruni...@chromium.org, devtools-...@chromium.org, mlippau...@chromium.org, oilpan-r...@chromium.org, v8-re...@googlegroups.com
Attention needed from Michael Lippautz
Dominik Inführ added 1 comment![]( "Open in Gerrit")
File src/profiler/heap-snapshot-generator.cc
Line 3466, Patchset 2 (Latest):
from->SetNamedAutoIndexReference(HeapGraphEdge::kWeak, nullptr, to,Michael Lippautz . unresolved
Why `SetNamedAutoIndexReference` vs `SetIndexedAutoIndexReference`?
Dominik Inführ
Edges just a have a name_or_index field and the edge type tells whether that field should be interpreted as name or index. But only kElement and kHidden edge types use the index, all other types including kWeak use the name. So we can't use `SetIndexedAutoIndexReference` here like below, we would need a kWeakElement or so instead. `SetNamedAutoIndexReference` solves this by turning the index into a string and using that as a name for that edge. I will add a comment for this.
Related details
Attention is currently required from:
- Michael Lippautz
Submit Requirements:
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Michael Lippautz (Gerrit)
May 11, 2026, 2:59:58 AMMay 11
to Dominik Inführ, v8-s...@luci-project-accounts.iam.gserviceaccount.com, android-bu...@system.gserviceaccount.com, Hannes Payer, cbruni...@chromium.org, devtools-...@chromium.org, mlippau...@chromium.org, oilpan-r...@chromium.org, v8-re...@googlegroups.com
Attention needed from Dominik Inführ
Michael Lippautz voted and added 1 comment![]( "Open in Gerrit")
Votes added by Michael Lippautz
| Code-Review | +1 |
1 comment
File src/profiler/heap-snapshot-generator.cc
Line 3466, Patchset 2:
from->SetNamedAutoIndexReference(HeapGraphEdge::kWeak, nullptr, to,Michael Lippautz . resolved
Dominik InführWhy `SetNamedAutoIndexReference` vs `SetIndexedAutoIndexReference`?
Edges just a have a name_or_index field and the edge type tells whether that field should be interpreted as name or index. But only kElement and kHidden edge types use the index, all other types including kWeak use the name. So we can't use `SetIndexedAutoIndexReference` here like below, we would need a kWeakElement or so instead. `SetNamedAutoIndexReference` solves this by turning the index into a string and using that as a name for that edge. I will add a comment for this.
Attention is currently required from:
- Dominik Inführ
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Dominik Inführ (Gerrit)
May 18, 2026, 12:58:53 AM (13 days ago) May 18
to Michael Lippautz, v8-s...@luci-project-accounts.iam.gserviceaccount.com, android-bu...@system.gserviceaccount.com, Hannes Payer, cbruni...@chromium.org, devtools-...@chromium.org, mlippau...@chromium.org, oilpan-r...@chromium.org, v8-re...@googlegroups.com
Dominik Inführ voted Commit-Queue+2![]( "Open in Gerrit")
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
v8-scoped@luci-project-accounts.iam.gserviceaccount.com (Gerrit)
May 18, 2026, 1:43:44 AM (13 days ago) May 18
to Dominik Inführ, Michael Lippautz, android-bu...@system.gserviceaccount.com, Hannes Payer, cbruni...@chromium.org, devtools-...@chromium.org, mlippau...@chromium.org, oilpan-r...@chromium.org, v8-re...@googlegroups.com
v8-s...@luci-project-accounts.iam.gserviceaccount.com submitted the change![]( "Open in Gerrit")
Unreviewed changes
3 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.
Change information
Commit message:
[heap, profiler] Emit weak edges for WeakMember in cppgc
This CL emits edges for WeakMember as weak edges. So far they were
emitted as strong edges. This will improve retaining paths and
dominators for cppgc objects.
Bug: 497855658
Change-Id: I08b3ea721785a590deaf33bd7d0992bd520c7a1a
Reviewed-by: Michael Lippautz <mlip...@chromium.org>
Commit-Queue: Dominik Inführ <dinf...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#107363}
Files:
- M include/v8-profiler.h
- M src/heap/cppgc-js/cpp-snapshot.cc
- M src/profiler/heap-snapshot-generator.cc
- M test/unittests/heap/cppgc-js/unified-heap-snapshot-unittest.cc
Change size: M
Delta: 4 files changed, 102 insertions(+), 7 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Michael Lippautz
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages