[profiler] Do not emit traced handles as roots [v8/v8 : main]
0 views
Skip to first unread message
Dominik Inführ (Gerrit)
Mar 25, 2026, 9:52:40 AM (6 days ago) Mar 25
to Michael Lippautz, Hannes Payer, AyeAye, V8 LUCI CQ, oilpan-r...@chromium.org, mlippau...@chromium.org, devtools-...@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)
Mar 26, 2026, 8:08:39 AM (5 days ago) Mar 26
to Dominik Inführ, Hannes Payer, AyeAye, V8 LUCI CQ, oilpan-r...@chromium.org, mlippau...@chromium.org, devtools-...@chromium.org, v8-re...@googlegroups.com
Attention needed from Dominik Inführ
Michael Lippautz voted and added 6 comments![]( "Open in Gerrit")
Votes added by Michael Lippautz
| Code-Review | +1 |
6 comments
Patchset-level comments
Michael Lippautz . resolved
lgtm
File src/handles/traced-handles.cc
Line 848, Patchset 8 (Latest):
ConservativeTracedHandlesNodeScanner::NodeFromConservativelyScannedPointer(Michael Lippautz . unresolved
`TryGetNodeFromInnerPointer`?
Line 860, Patchset 8 (Latest):
TracedNode* ConservativeTracedHandlesNodeScanner::FindNode(Michael Lippautz . unresolved
`TryFindNode`?
Line 861, Patchset 8 (Latest):
const void* address) const {Michael Lippautz . unresolved
nit: `hint`?
File src/heap/cppgc-js/cpp-snapshot.cc
Line 1031, Patchset 8 (Latest):
if (TracedNode* node = traced_handles_scanner_.FindNode(address)) {
Tagged<Object> object = node->object();
if (IsHeapObject(object)) {
graph_builder_.VisitTracedReferenceOnStack(
traced_handles_parent_scope_.ParentAsRootState(),
Cast<HeapObject>(object));
}
}Michael Lippautz . resolved
Nice!
File src/profiler/heap-snapshot-generator.cc
Line 2663, Patchset 8 (Parent):
heap_->IterateWeakGlobalHandles(&extractor);Michael Lippautz . unresolved
Can we remove the method? If not, what is it used for (and does this make sense)?
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)
Mar 26, 2026, 8:24:59 AM (5 days ago) Mar 26
to Michael Lippautz, Hannes Payer, AyeAye, V8 LUCI CQ, oilpan-r...@chromium.org, mlippau...@chromium.org, devtools-...@chromium.org, v8-re...@googlegroups.com
Attention needed from Michael Lippautz
Dominik Inführ added 5 comments![]( "Open in Gerrit")
Patchset-level comments
File-level comment, Patchset 9 (Latest):
Dominik Inführ . resolved
Thanks for the review! Lost the +1, PTALA
File src/handles/traced-handles.cc
Line 848, Patchset 8:
ConservativeTracedHandlesNodeScanner::NodeFromConservativelyScannedPointer(Michael Lippautz . resolved
Dominik Inführ`TryGetNodeFromInnerPointer`?
Done
Line 860, Patchset 8:
TracedNode* ConservativeTracedHandlesNodeScanner::FindNode(Michael Lippautz . resolved
Dominik Inführ`TryFindNode`?
Done
Line 861, Patchset 8:
const void* address) const {Michael Lippautz . resolved
Dominik Inführnit: `hint`?
Done
File src/profiler/heap-snapshot-generator.cc
Line 2663, Patchset 8 (Parent):
heap_->IterateWeakGlobalHandles(&extractor);Michael Lippautz . resolved
Can we remove the method? If not, what is it used for (and does this make sense)?
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. |
Dominik Inführ (Gerrit)
Mar 26, 2026, 9:40:03 AM (5 days ago) Mar 26
to Michael Lippautz, Hannes Payer, AyeAye, V8 LUCI CQ, oilpan-r...@chromium.org, mlippau...@chromium.org, devtools-...@chromium.org, v8-re...@googlegroups.com
Attention needed from Michael Lippautz
Dominik Inführ voted![]( "Open in Gerrit")
| Auto-Submit | +1 |
| Commit-Queue | +1 |
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)
Mar 26, 2026, 9:43:01 AM (5 days ago) Mar 26
to Dominik Inführ, Hannes Payer, AyeAye, V8 LUCI CQ, oilpan-r...@chromium.org, mlippau...@chromium.org, devtools-...@chromium.org, v8-re...@googlegroups.com
Attention needed from Dominik Inführ
Michael Lippautz voted![]( "Open in Gerrit")
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. |
V8 LUCI CQ (Gerrit)
Mar 26, 2026, 9:45:11 AM (5 days ago) Mar 26
to Dominik Inführ, Michael Lippautz, Hannes Payer, AyeAye, oilpan-r...@chromium.org, mlippau...@chromium.org, devtools-...@chromium.org, v8-re...@googlegroups.com
V8 LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
Commit message:
[profiler] Do not emit traced handles as roots
Traced handles were emitted as weak roots. We do not even want to
emit traced handles even as weak roots. The CL removes them as root
but adds support for traced handles found on the stack during
conservative stack scanning.
Bug: 492402253
Change-Id: I1253043204c7dadf331aa7a731d04b127110c054
Reviewed-by: Michael Lippautz <mlip...@chromium.org>
Commit-Queue: Michael Lippautz <mlip...@chromium.org>
Auto-Submit: Dominik Inführ <dinf...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#106071}
Files:
- M src/handles/traced-handles.cc
- M src/handles/traced-handles.h
- M src/heap/cppgc-js/cpp-snapshot.cc
- M src/heap/heap.cc
- M src/heap/heap.h
- M src/heap/traced-handles-marking-visitor.cc
- M src/heap/traced-handles-marking-visitor.h
- M src/profiler/heap-snapshot-generator.cc
- M test/unittests/heap/cppgc-js/unified-heap-snapshot-unittest.cc
Change size: M
Delta: 9 files changed, 144 insertions(+), 40 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