[iOS] Flush layer pool after platform view dispose #54056
Conversation
cbracken
force-pushed
the
flush-unused-layers
branch
from
20da635
to
8e2bac7
Compare
`FlutterPlatformViewLayerPool` is a pool of iOS layers used for rendering platform views on iOS. When layers are no longer needed, we currently mark them available for re-use but we never actually flush them and thus recover the memory associated with these layers once we know that the platformview is no longer used. We now flush the layer pool to recover unused memory after platform views have been disposed. See: flutter/flutter#152053
cbracken
force-pushed
the
flush-unused-layers
branch
from
68e66b3
to
fe05e81
Compare
|
Taking a look at how to sneak a test in here. |
|
LGTM |
|
Added a test. |
cbracken
force-pushed
the
flush-unused-layers
branch
from
cd52aef
to
b5ab12e
Compare
| auto pool = flutter::FlutterPlatformViewLayerPool{}; | ||
|
|
||
| // Add layers to the pool. | ||
| auto layer1 = pool.GetLayer(gr_context.get(), ios_context, MTLPixelFormatBGRA8Unorm); |
There was a problem hiding this comment.
Is there a way to check this is actually released? Or is that not a concern? I know the __weak/@autoreleasepool pattern won't work with non-Obj-C objects.
There was a problem hiding this comment.
The pool holds these in std::shared_ptr<FlutterPlatformViewLayer> so they'll be cleaned up via RAII. If we templatized FlutterPlatformViewLayerPool we could stick in some other class and manipulate a count in the ctor/dtor that we could then test against, but that probably wouldn't give us much more benefit, since it still wouldn't show that the real FlutterPlatformView layer properly releases its memory -- it uses C smart pointers for all but the GrDirectContext so I trust it, but it's still not tested.
An integration test that looks at overall memory is probably the way to do something like that.
Since this method isn't mutating the pool, we should mark it const. Cleanup spotted while working on #54056. No test because no semantic change. The compiler is the test. [C , Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
cbracken
added
the
autosubmit
FlutterPlatformViewLayerPoolis a pool of iOS layers used for rendering platform views on iOS. When layers are no longer needed, we currently mark them available for re-use but we never actually flush them and thus recover the memory associated with these layers once we know that the layers are no longer needed.We now flush the layer pool on
SubmitFrameif a previously-used layer is no longer used in the current frame. In theory, this could cause a performance regression in the case where an additional layer is needed every second or third frame, but we should keep it simple on the first pass and only complicate things later if warranted.Fixes: flutter/flutter#152053
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.