[heap] Remove visibility SCC pass [v8/v8 : main]
Dominik Inführ (Gerrit)
Dominik Inführ voted and added 1 comment![]( "Open in Gerrit")
Votes added by Dominik Inführ
| Commit-Queue | +1 |
1 comment
Related details
- Michael Lippautz
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 added 1 comment![]( "Open in Gerrit")
Note that I added tests for WeakContainers in this CL: https://chromium-review.git.corp.google.com/c/v8/v8/+/7817336.
Related details
- Michael Lippautz
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Michael Lippautz (Gerrit)
Michael Lippautz added 3 comments![]( "Open in Gerrit")
heap and adds their edges. But for this pass we need to know whetherI don't understand this sentence
#include "src/heap/marking-worklist-inl.h"Since you are here: More includes to clean up?
// (4) Add persistent roots.Would it be problematic to move that into the front as well? Basically cluster all roots.
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 5 comments![]( "Open in Gerrit")
I should've addressed all comments now. I also changed how we trace weak container - see the comment I added for this on the CL.
heap and adds their edges. But for this pass we need to know whetherI don't understand this sentence
I updated this. Should be better now.
Since you are here: More includes to clean up?
I removed the includes that were suggested as not used.
if (weak_desc && !graph_builder_.IsReachableFromStack(header)) {Note that I changed this here as well. We simply call the regular Trace() method if the weak container is on the stack. This visitor strongifies the key itself (both for the unit test and the HeapHashTableBacking in production) so we don't need to add that strong edge to the key explicitly in VisitEphemeron() anymore.
Would it be problematic to move that into the front as well? Basically cluster all roots.
- 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 2 comments![]( "Open in Gerrit")
Votes added by Michael Lippautz
| Code-Review | +1 |
2 comments
As discussed offline, this seems to be actually okay in terms of how many nodes we show and snapshot sizes we produce.
if (weak_desc && !graph_builder_.IsReachableFromStack(header)) {Note that I changed this here as well. We simply call the regular Trace() method if the weak container is on the stack. This visitor strongifies the key itself (both for the unit test and the HeapHashTableBacking in production) so we don't need to add that strong edge to the key explicitly in VisitEphemeron() anymore.
I think this is also what the regular marker does. It will pick up the strong Trace() method for any object found on the stack.
Related details
- 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
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] Remove visibility SCC pass
Because "expose internals" is now always enabled, the visibility
filtering pass in cpp-snapshot.cc doesn't filter out any objects
anymore. This CL therefore drops this pass, which also allows us
to drop StateBase and its derived classes.
There is one main pass which iterates all the cppgc objects in the
heap and adds their edges. In order to do this pass we need to
compute some additional information beforehand. We need to know:
* Whether object is weak container (which requires additional heap
iteration). Weak containers are visited using their weak tracing
callback.
* And whether the object is reachable from stack. This is again needed
for weak containers. The weak container is visited strongly in this
case.
- M src/heap/cppgc-js/cpp-snapshot.cc
Code-Review: +1 by Michael Lippautz
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |