Skip to content

Rasterizer regression with latest Skia roll #12250

Closed

Description

@goderbauer

flutter-team-archive/engine#4131 regressed some of our rasterizer benchmarks, namely:

  • complex_layout_scroll_perf__timeline_summary/frames missed_frame_rasterizer_budget_count
  • complex_layout_scroll_perf__timeline_summary/average_frame_rasterizer_time_millis
  • futter_gallery__transition_perf/worst_frame_rasterizer_time_millis

/cc @jason-simmons @chinmaygarde

Activity

  1. added
    c: performanceRelates to speed or footprint issues (see "perf:" labels)
    engineflutter/engine related. See also e: labels.
    c: regressionIt was better in the past than it is now
    on Sep 25, 2017
  2. self-assigned this
    on Sep 26, 2017
  3. jason-simmons commented on Sep 26, 2017

    @jason-simmons
    Member

    Some changes in this engine that may have affected rasterizer performance:

    • Using the "low" SkFilterQuality instead of the default "none" when drawing raster cache images (flutter-team-archive/engine@191a2fa)
    • The GrTextureOp implementation of SkGpuDevice::drawImage in Skia.
      • The first version of this caused significant regressions on some benchmarks. It's been modified for improved performance on certain GPUs (e.g. google/skia@4127456) but might still impact performance on mobile.
  4. chinmaygarde commented on Sep 26, 2017

    @chinmaygarde
    Contributor

    I am having a tough time trying to reproduce the numbers in the benchmarks. I have tried to collect traces and measure the time spent in GPURasterizer::Draw both before and after the roll. Two devices were use for this, the Nexus 5 (Adreno 330) and the Moto G4 (Adreno 405). The scene was the complex layout demo where I scrolled down and then back up. On both devices, the new numbers are faster.

    New-MotoG4-ShouldBeSlower.json.zip
    New-Nexus5-ShouldBeSlower.json.zip
    Old-MotoG4-ShouldBeFaster.json.zip
    Old-Nexus5-ShouldBeFaster.json.zip

  5. Hixie commented on Sep 26, 2017

    @Hixie
    Contributor

    @chinmaygarde I recommend running the devicelab test directly on your workstation.

  6. jason-simmons commented on Sep 27, 2017

    @jason-simmons
    Member

    I can reproduce this by running the devicelab complex_layout_scroll_perf__timeline_summary benchmark with a local build of the engine on a Moto G4.

    The issue seems to be that the new GrTextureOp implementation of drawImageRect is slow on some GPUs. Most of the performance difference disappears on the G4 if I replace SkGpuDevice::drawImageRect with the version from google/skia@a184ac7 that calls drawTextureProducer.

    The performance is sensitive to the specific GPU. I didn't see a performance difference on a Pixel XL (with an Adreno 530)

  7. jason-simmons commented on Sep 27, 2017

    @jason-simmons
    Member

    I'm also seeing a decrease in benchmark performance with the new SkGpuDevice::drawImageRect implementation on a Samsung device with a Mali-T760 GPU

    @bsalomon

  8. Hixie commented on Sep 28, 2017

    @Hixie
    Contributor

    Given that this is GPU-specific, maybe we should send that feedback to Skia and remove the TODAY milestone (meaning we don't think this is critical for us), and rebaseline the relevant metrics.

  9. bsalomon commented on Oct 6, 2017

    @bsalomon
    Contributor

    I was able to get my hands on a G4 and reproduce this. I am seeing ~6.25 ms with the old code path and ~8.25 with the new code path. The code path also takes tends to have 1 more missed frame.

  10. bsalomon commented on Oct 6, 2017

    @bsalomon
    Contributor

    If I take the new code path (GrTextureOp) but disallow batching ops with more than 2 different images then I am able to recover the performance.

  11. bsalomon commented on Oct 9, 2017

    @bsalomon
    Contributor

    Correction, the performance is only recovered if there is no batching of multiple textures. Interestingly, a Skia mircobenchmark that draws images shows a 2x improvement by batching different textures on the same device. I'm investigating why this disparity exists.

  12. bsalomon commented on Oct 12, 2017

    @bsalomon
    Contributor

    It turns out that the size of the rendered rectangles (not the image) determines whether batching multiple images into a single draw is a performance win or not. It's a win for smaller rectangles and a loss for larger. In this flutter benchmark the cached raster content that is being composited exceeds the crossover point on the Adreno 405. I'm gathering data across a wide range of GPUs using an updated Skia benchmark in order to determine how to move forward.

  13. chinmaygarde commented on Nov 6, 2017

    @chinmaygarde
    Contributor

    Based on our conversation, the conclusion here is that the changes in Skia cause performance improvements on some devices while minor regressions in others. The way in which batching affects performance also seems to differ between different members of the same processor family. @bsalomon is working on fine tuning the heuristics further and those changes will land in the upcoming days and weeks. The baseline has been updated for this benchmark for now and there are no more actionable items to be taken on this front in Flutter.

  14. added a commit that references this issue on Sep 12, 2019
  15. added a commit that references this issue on Sep 12, 2019
  16. added a commit that references this issue on Sep 13, 2019
  17. added a commit that references this issue on Sep 13, 2019
  18. added 3 commits that reference this issue on Sep 30, 2019
  19. github-actions commented on Sep 4, 2021

    @github-actions

    This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

  20. locked as resolved and limited conversation to collaborators on Sep 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

c: performanceRelates to speed or footprint issues (see "perf:" labels)c: regressionIt was better in the past than it is nowdependency: skiaSkia team may need to help usengineflutter/engine related. See also e: labels.

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions