-
Notifications
You must be signed in to change notification settings - Fork 6k
[fuchsia] Enable raster cache on Fuchsia #17753
Conversation
40182ac
to
17c21cd
Compare
I've verified that this fixes all raster cache problems I'm seeing on Fuchsia. Any good reasons for why this cache was disabled on Fuchsia? I can't tell from the history of the code. |
17c21cd
to
6286f96
Compare
Did some more testing and it turns out that simply enabling the raster cache results in serious performance problems on Fuchsia. Afaict, the reasons for that are:
I think 1) will be fixed as we switch to embedder API for Fuchsia integration. 2) will need some tuning but is also not going to be as critical to have right when a gr_context exists as the cost of populating the cache is no longer much slower than drawing uncached picture. This change has been updated to only enable the cache for small picture layers that can be efficiently rasterized without a gr_context. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This workaround looks reasonable.
On non-Fuchsia platforms, the raster cache performance is mainly covered by complex_layout_scroll_perf*
devicelab benchmark (scroll_perf_test.dart). @godofredoc @iskakaushik : I wonder if or when our devicelab could have a Fuchsia device running such performance test to track our progress in Fuchsia raster cache?
No need to review this right now as I'm not sure it's something we want to land. This is more of a heads as I'm trying to collect information for next steps. I'm currently looking at making some temporary changes to add gr_context support and enable the cache properly. That might be something worth landing if it doesn't turn out to be too complicated. |
6286f96
to
882e1b4
Compare
Latest changes pass a gr_context to ScopedFrame instances and that makes us efficiently populating the cache. This is now turning on the full functionality of the raster cache and it has been verified to provide a performance improvement. Cache size in vulkan_surface_producer.cc likely need some adjustments but I prefer to do that in a separate change. PTAL. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
882e1b4
to
1c471b0
Compare
The raster cache is critical for good performance. This enables the cache and provides a GrContext to ScopedFrame instances so the cache can be efficiently populated. Small increase in peak GPU memory usage is expected from this change. Otherwise, no change in behavior expected. Fixes flutter/flutter#54950
1c471b0
to
5010730
Compare
The raster cache is critical for good performance. This
enables the cache and provides a GrContext to ScopedFrame
instances so the cache can be efficiently populated.
Small increase in peak GPU memory usage is expected from
this change. Otherwise, no change in behavior expected.
Fixes flutter/flutter#54950