You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Rasterizer regression with latest Skia roll #12250
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.
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.
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)
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.
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.
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.
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.
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.
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.
Activity
jason-simmons commentedon Sep 26, 2017
Some changes in this engine that may have affected rasterizer performance:
chinmaygarde commentedon Sep 26, 2017
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::Drawboth 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
Hixie commentedon Sep 26, 2017
@chinmaygarde I recommend running the devicelab test directly on your workstation.
jason-simmons commentedon Sep 27, 2017
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)
jason-simmons commentedon Sep 27, 2017
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
Hixie commentedon Sep 28, 2017
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.
bsalomon commentedon Oct 6, 2017
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.
bsalomon commentedon Oct 6, 2017
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.
bsalomon commentedon Oct 9, 2017
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.
bsalomon commentedon Oct 12, 2017
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.
chinmaygarde commentedon Nov 6, 2017
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.
Roll engine from 7ea9884..3c6383f (77 commits 🔥) (#40372)
Roll engine to 6e6de94
Roll engine to 6e6de94 (#40394)
Roll engine 7ea9884..1b63444 (91 commits 🔥🔥🔥) (#40442)
Roll engine from 7ea9884..3c6383f (77 commits 🔥) (flutter#40372)
Roll engine to 6e6de94 (flutter#40394)
Roll engine 7ea9884..1b63444 (91 commits 🔥🔥🔥) (flutter#40442)
github-actions commentedon Sep 4, 2021
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 -vand a minimal reproduction of the issue.