Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Reduce layer tree pipeline depth to 2.#3909

Merged
chinmaygarde merged 1 commit into
flutter-team-archive:masterfrom
chinmaygarde:master
Jul 20, 2017
Merged

Reduce layer tree pipeline depth to 2.#3909
chinmaygarde merged 1 commit into
flutter-team-archive:masterfrom
chinmaygarde:master

Conversation

@chinmaygarde

Copy link
Copy Markdown
Contributor

Setting the pipeline depth to three solves no use cases and causes the effects of jank to be exacerbated. Consider the following:

  • Before the start of an animation of a particularly expensive scene, the pipeline is completely empty. The scene is cheap to represent as an SkPicture on UI thread but the GPU thread operations for the same are expensive and cannot be completed in one frame interval.
  • The UI thread finishes submission on an SkPicture to the GPU thread and schedules another frame.
  • Since the pipeline only has one item it, the request for the second frame goes through. At the next frame interval, the UI thread starts constructing the second SkPicture. The timestamp it uses to step on interpolators taking part in the animation is one frame interval.
  • The UI thread then submits this SkPicture to the GPU thread. The number of items in the pipeline is 2 and so another frame request is accepted and serviced at the next frame interval.
  • But the first SkPicture submitted by the UI thread has still not been processed by the GPU thread. It takes a while but the GPU thread eventually gets to processing the first SkPicture.
  • The second picture the GPU thread processes has been prepared (on the UI thread) with interpolators that assume a single frame interval work of delay from the frame before it. However, the GPU thread ends up servicing this second SkPicture at an interval that is well over a frame interval.
  • This problem becomes worse when this same sequence of events is repeated for the third picture in the pipeline.
  • If the GPU frame time eventually settles down do to the raster cache, the same difference in the rates at which frames are produced on the UI thread and consumed on the GPU thread causes more unnatural jank.

We do need some pipelining because we want to be able to simultaneously consume one item on the GPU thread while another one is being produced on the UI thread.

But we don't want excess room in the pipeline to cause the UI thread to fill up the pipeline with pictures for intervals (as determined by interpolators) that are different from the ones that the GPU thread will consume the SkPictures.

So reduce the pipeline depth to 2.

Traces attached: PipelineTo2.json.zip

@Hixie

Hixie commented Jul 20, 2017

Copy link
Copy Markdown
Contributor

RSLGTM

@chinmaygarde
chinmaygarde merged commit 5fcfb99 into flutter-team-archive:master Jul 20, 2017
@eseidelGoogle

Copy link
Copy Markdown
Contributor

FYI @jamesr @abarth

@Hixie

Hixie commented Jul 21, 2017

Copy link
Copy Markdown
Contributor

FYI this landed as part of flutter/flutter#11318 which regressed flutter_gallery_ios__transition_perf average_frame_rasterizer_time_millis by about 20%.

@Hixie

Hixie commented Jul 21, 2017

Copy link
Copy Markdown
Contributor

Of the three patches that were in that roll, this is the most likely candidate.

There's no obvious impact on any other benchmark.

@Hixie

Hixie commented Jul 25, 2017

Copy link
Copy Markdown
Contributor

@chinmaygarde Did you have a chance to investigate the performance regression?

@chinmaygarde

Copy link
Copy Markdown
Contributor Author

I haven't as I am still investigating the color correctness issue. Though I don't see how this patch can affect the average frame rasterization time. This controls how many frames are in the pipeline, not how long they take to rasterize.

@Hixie

Hixie commented Jul 26, 2017

Copy link
Copy Markdown
Contributor

Yeah I don't understand it either.

@eseidelGoogle

Copy link
Copy Markdown
Contributor

Would it affect CPU contention? Could having a larger pipeline have allowed the frames to render at more open cpu times?

@j9brown

j9brown commented Jul 26, 2017 via email

Copy link
Copy Markdown

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Development

Successfully merging this pull request may close these issues.

6 participants