[heap, profiler] Avoid EmbedderGraph API for cpp-snapshot.cc [v8/v8 : main]
Michael Lippautz (Gerrit)
Michael Lippautz voted and added 1 comment![]( "Open in Gerrit")
Votes added by Michael Lippautz
| Code-Review | +1 |
1 comment
PrePassIterator pre_pass_iterator(For future CLs: Please do not add merging phases into larger refactorings. I think this is okay here but I was surprised to see the actual algorithm being changed as well.
Related details
- Dominik Inführ
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)
Dominik Inführ added 2 comments![]( "Open in Gerrit")
PTALA. I rebased this CL and added more tests. I also removed a DCHECK I added in this CL again. One test triggered that DCHECK.
PrePassIterator pre_pass_iterator(For future CLs: Please do not add merging phases into larger refactorings. I think this is okay here but I was surprised to see the actual algorithm being changed as well.
Sorry, I should have made this more clear when posting the CL. This change was necessary to keep merging of wrappers working as expected. We now immediately create HeapEntries in LiveObjectsIterator, previously we only created EmbedderNodes there. So now we need to know earlier what objects we need to merge sooner. We even need to know all wrappers before iterating the stack. Because during stack iteration we already create HeapEntries. That's why I needed to add this to the pre-pass.
Related details
- Michael Lippautz
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)
Michael Lippautz voted and added 1 comment![]( "Open in Gerrit")
Votes added by Michael Lippautz
| Code-Review | +1 |
1 comment
PrePassIterator pre_pass_iterator(Dominik InführFor future CLs: Please do not add merging phases into larger refactorings. I think this is okay here but I was surprised to see the actual algorithm being changed as well.
Sorry, I should have made this more clear when posting the CL. This change was necessary to keep merging of wrappers working as expected. We now immediately create HeapEntries in LiveObjectsIterator, previously we only created EmbedderNodes there. So now we need to know earlier what objects we need to merge sooner. We even need to know all wrappers before iterating the stack. Because during stack iteration we already create HeapEntries. That's why I needed to add this to the pre-pass.
- Dominik Inführ
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Dominik Inführ (Gerrit)
Dominik Inführ voted and added 1 comment![]( "Open in Gerrit")
Votes added by Dominik Inführ
| Commit-Queue | +2 |
1 comment
Related details
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)
v8-s...@luci-project-accounts.iam.gserviceaccount.com submitted the change![]( "Open in Gerrit")
Change information
[heap, profiler] Avoid EmbedderGraph API for cpp-snapshot.cc
So far cpp-snapshot.cc was used through the EmbedderGraph public API
from the heap snapshot generator. This CL avoids going through that
API, so that cpp-snapshot.cc can directly create HeapEntries instead
of EmbedderNodes.
- M src/DEPS
- M src/heap/cppgc-js/cpp-heap.cc
- M src/heap/cppgc-js/cpp-snapshot.cc
- M src/heap/cppgc-js/cpp-snapshot.h
- M src/profiler/heap-profiler.cc
- M src/profiler/heap-profiler.h
- M src/profiler/heap-snapshot-generator.cc
- M src/profiler/heap-snapshot-generator.h
- M test/unittests/heap/cppgc-js/unified-heap-snapshot-unittest.cc
- M test/unittests/profiler/heap-snapshot-utils.cc
- M test/unittests/profiler/heap-snapshot-utils.h
Code-Review: +1 by Michael Lippautz
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |