bindings/heap: Move GC prologue callbacks to ThreadState [chromium/src : main]
Michael Lippautz (Gerrit)
Michael Lippautz voted and added 3 comments![]( "Open in Gerrit")
Votes added by Michael Lippautz
| Commit-Queue | +1 |
3 comments
Hey Kentaro, I get asked repeatedly how GC is wired up in Blink. One feedback I got there is that it's confusing that things are often split apart for historical reasons and I have to agree. This CL here moves the GC callbacks into ThreadState to consolidate that setup. It also makes it easier to follow ASWManager. There's only one callback that requires `core/` (DevTools) which we can probably fix with a proper observer when it's needed.
v8::EmbedderGraph::Node::Detachedness V8GCController::DetachednessFromWrapper(I couldn't think of a good place for this one. It's not "GC controller" but just used for heap snapshots at this point. Maybe just putting it in `v8_initializer.cc` would be good enough.
(It needs to stay in `core/` because of `Node`).
DevToolsCountersCallback dev_tools_counters_callback_;This callback (devtools tracing scope) requires `core/`. I was asked already if we can make a GC observer at some point which we should probably do when there's more uses.
Related details
- Kentaro Hara
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Kentaro Hara (Gerrit)
Kentaro Hara voted and added 3 comments![]( "Open in Gerrit")
Votes added by Kentaro Hara
| Code-Review | +1 |
3 comments
LGTM
v8::EmbedderGraph::Node::Detachedness V8GCController::DetachednessFromWrapper(I couldn't think of a good place for this one. It's not "GC controller" but just used for heap snapshots at this point. Maybe just putting it in `v8_initializer.cc` would be good enough.
(It needs to stay in `core/` because of `Node`).
Acknowledged
DevToolsCountersCallback dev_tools_counters_callback_;This callback (devtools tracing scope) requires `core/`. I was asked already if we can make a GC observer at some point which we should probably do when there's more uses.
+1 to introduce an observer pattern only when we have multiple cases.
Related details
- Michael Lippautz
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
| 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
| Commit-Queue | +2 |
1 comment
DevToolsCountersCallback dev_tools_counters_callback_;Kentaro HaraThis callback (devtools tracing scope) requires `core/`. I was asked already if we can make a GC observer at some point which we should probably do when there's more uses.
+1 to introduce an observer pattern only when we have multiple cases.
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chromium LUCI CQ (Gerrit)
Chromium LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
bindings/heap: Move GC prologue callbacks to ThreadState
ThreadState is already the home for anything garbage collection
related in Blink. It combines the various GC related calls and ways to
attach GC to an Isolate for custom logic around some weakness
clearing.
The CL moves the prologue and epilogue callbacks to ThreadState as
well to unify the place where GC behavior is customized. This makes it
easier to find and explain where GC is configured in Blink.
- M third_party/blink/renderer/bindings/bindings.gni
- M third_party/blink/renderer/bindings/core/v8/active_script_wrappable.h
- D third_party/blink/renderer/bindings/core/v8/v8_embedder_graph_builder.cc
- D third_party/blink/renderer/bindings/core/v8/v8_embedder_graph_builder.h
- M third_party/blink/renderer/bindings/core/v8/v8_gc_controller.cc
- M third_party/blink/renderer/bindings/core/v8/v8_initializer.cc
- M third_party/blink/renderer/core/html/forms/html_input_element.cc
- M third_party/blink/renderer/core/layout/layout_object.cc
- M third_party/blink/renderer/platform/bindings/active_script_wrappable_base.cc
- M third_party/blink/renderer/platform/bindings/active_script_wrappable_base.h
- M third_party/blink/renderer/platform/bindings/script_forbidden_scope.h
- M third_party/blink/renderer/platform/bindings/v8_per_isolate_data.cc
- M third_party/blink/renderer/platform/bindings/v8_per_isolate_data.h
- M third_party/blink/renderer/platform/heap/thread_state.cc
- M third_party/blink/renderer/platform/heap/thread_state.h
- M third_party/blink/renderer/platform/testing/main_thread_isolate.cc
Code-Review: +1 by Kentaro Hara
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |