这是indexloc提供的服务,不要输入任何密码
Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

[fuchsia] Enable raster cache on Fuchsia #17753

Merged
merged 1 commit into from
Apr 20, 2020

Conversation

dreveman
Copy link
Contributor

@dreveman dreveman commented Apr 16, 2020

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

@auto-assign auto-assign bot requested a review from iskakaushik April 16, 2020 11:07
@dreveman dreveman force-pushed the fuchsia-enable-raster-cache branch from 40182ac to 17c21cd Compare April 16, 2020 11:09
@dreveman dreveman changed the title [fuchsia] Enable raster cache on Fuchsia (#54950) [fuchsia] Enable raster cache on Fuchsia Apr 16, 2020
@dreveman
Copy link
Contributor Author

@chinmaygarde @liyuqian

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.

@dreveman dreveman force-pushed the fuchsia-enable-raster-cache branch from 17c21cd to 6286f96 Compare April 16, 2020 13:43
@dreveman dreveman changed the title [fuchsia] Enable raster cache on Fuchsia [fuchsia] Enable limited use of raster cache on Fuchsia Apr 16, 2020
@dreveman
Copy link
Contributor Author

@chinmaygarde @liyuqian

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.

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:

  1. gr_context is missing on Fuchsia so we end up using software rendering to populate the cache.
  2. vulkan surface pool limits are too low.

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.

@liyuqian liyuqian added perf: speed Performance issues related to (mostly rendering) speed severe: performance Relates to speed or footprint issues. labels Apr 16, 2020
Copy link
Contributor

@liyuqian liyuqian left a 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?

@dreveman
Copy link
Contributor Author

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.

@dreveman dreveman force-pushed the fuchsia-enable-raster-cache branch from 6286f96 to 882e1b4 Compare April 17, 2020 15:01
@dreveman dreveman changed the title [fuchsia] Enable limited use of raster cache on Fuchsia [fuchsia] Enable raster cache on Fuchsia Apr 17, 2020
@dreveman
Copy link
Contributor Author

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.

@dreveman dreveman requested a review from liyuqian April 17, 2020 15:08
Copy link
Contributor

@liyuqian liyuqian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dreveman dreveman force-pushed the fuchsia-enable-raster-cache branch from 882e1b4 to 1c471b0 Compare April 17, 2020 20:33
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
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes perf: speed Performance issues related to (mostly rendering) speed severe: performance Relates to speed or footprint issues.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Poor performance on Fuchsia due to raster cache being disabled
5 participants