Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Impeller] Don't decompress into device buffer for Vulkan/GLES. #43493

Merged
merged 4 commits into from
Jul 10, 2023

Conversation

jonahwilliams
Copy link
Member

@jonahwilliams jonahwilliams commented Jul 9, 2023

My observations on the Pixel 6 device are that performing device allocations from multiple threads can dramatically slow down the raster task workload. As a stopgap solution, we can adjust image upload to only touch the device allocator on the IO thread which reduces the parallel access.

This doesn't have any impact on the S10, but locally on the Pixel 6 it is a night and day difference. I am testing using jonahwilliams/forked_gallery and navigating to the Reply demo. This demo has a large number of images, several of which are quite large.

Work towards flutter/flutter#129392

Before

Page transition is ~4 frames.

image

After

Page transition is ~20 frames.

image

@chinmaygarde chinmaygarde changed the title [Impeller] dont decompress into device buffer for Vulkan/GLES [Impeller] Don't decompress into device buffer for Vulkan/GLES. Jul 10, 2023
@jonahwilliams jonahwilliams marked this pull request as ready for review July 10, 2023 19:57
context, bitmap_result.sk_bitmap, gpu_disabled_switch,
impeller::StorageMode::kDevicePrivate,
Copy link
Member Author

Choose a reason for hiding this comment

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

This works because both GLES and Vulkan don't really have a host visible abstraction and always do an upload.

@jonahwilliams jonahwilliams added the autosubmit Merge PR when tree becomes green via auto submit App label Jul 10, 2023
@auto-submit auto-submit bot merged commit 467e5ea into flutter:main Jul 10, 2023
26 checks passed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 10, 2023
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Jul 10, 2023
…130296)

flutter/engine@7d054ab...2a0dd9d

2023-07-10 chinmaygarde@google.com [Impeller] Fix Vulkan validation error when there is a blit directed at a swapchain image. (flutter/engine#43527)
2023-07-10 skia-flutter-autoroll@skia.org Roll Skia from 89313a8cadab to aad8fbb17d69 (3 revisions) (flutter/engine#43526)
2023-07-10 skia-flutter-autoroll@skia.org Roll Skia from c06d7ef5c276 to 89313a8cadab (2 revisions) (flutter/engine#43521)
2023-07-10 jonahwilliams@google.com [Impeller] Don't decompress into device buffer for Vulkan/GLES. (flutter/engine#43493)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC aaclarke@google.com,rmistry@google.com,zra@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
kjlubick pushed a commit to kjlubick/engine that referenced this pull request Jul 14, 2023
…ter#43493)

My observations on the Pixel 6 device are that performing device allocations from multiple threads can dramatically slow down the raster task workload. As a stopgap solution, we can adjust image upload to only touch the device allocator on the IO thread which reduces the parallel access.

This doesn't have any impact on the S10, but locally on the Pixel 6 it is a night and day difference. I am testing using jonahwilliams/forked_gallery and navigating to the Reply demo. This demo has a large number of images, several of which are quite large.

Work towards flutter/flutter#129392

### Before

Page transition is ~4 frames.

![image](https://github.com/flutter/engine/assets/8975114/b6d1c225-060b-4a20-9737-ad668423799a)

### After

Page transition is ~20 frames.

![image](https://github.com/flutter/engine/assets/8975114/5ff1f857-8327-4d04-b40a-3da4a5fc91a4)
auto-submit bot pushed a commit that referenced this pull request Oct 23, 2023
Testing locally I cant see any impact from this. Its possible the problem this "fixed" was instead fixed by affinity changes and now its no longer necessary.

See #43493 .
harryterkelsen pushed a commit that referenced this pull request Oct 23, 2023
Testing locally I cant see any impact from this. Its possible the problem this "fixed" was instead fixed by affinity changes and now its no longer necessary.

See #43493 .
gbtb16 pushed a commit to gbtb16/flutter that referenced this pull request Nov 6, 2023
…lutter#130296)

flutter/engine@7d054ab...2a0dd9d

2023-07-10 chinmaygarde@google.com [Impeller] Fix Vulkan validation error when there is a blit directed at a swapchain image. (flutter/engine#43527)
2023-07-10 skia-flutter-autoroll@skia.org Roll Skia from 89313a8cadab to aad8fbb17d69 (3 revisions) (flutter/engine#43526)
2023-07-10 skia-flutter-autoroll@skia.org Roll Skia from c06d7ef5c276 to 89313a8cadab (2 revisions) (flutter/engine#43521)
2023-07-10 jonahwilliams@google.com [Impeller] Don't decompress into device buffer for Vulkan/GLES. (flutter/engine#43493)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC aaclarke@google.com,rmistry@google.com,zra@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autosubmit Merge PR when tree becomes green via auto submit App
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants