Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2567253002:
arc: Avoid the resize operation in GetImageForScale() whenever possible (Closed)
|
Created:
4 years ago by Yusuke Sato Modified:
4 years ago CC:
chromium-reviews, elijahtaylor+arcwatch_chromium.org, yusukes+watch_chromium.org, tfarina, hidehiko+watch_chromium.org, lhchavez+watch_chromium.org, Matt Giuca Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
Descriptionarc: Avoid the resize operation in GetImageForScale() whenever possible
to remove UI jank.
ArcAppIcon::Source::GetImageForScale() is one of the lowest level
functions for the ARC app icon stuff which can easily be called
thousands of times while e.g. the user enters a few characters to
the app search box.
However, the function does a relatively heavy Skia operation,
gfx::ImageSkiaOperations::CreateResizedImage() with RESIZE_BEST,
against either IDR_APP_DEFAULT_ICON or IDR_ARC_SUPPORT_ICON_NN,
and b/33377533 shows that the operation actually slows down Chrome
OS UI significantly.
For example, on a recent Chromebook, pressing Search, then entering
pressing characters to the app search box freezes the browser's UI
thread for about (5 * num_of_arc_apps * num_of_key_presses)
milliseconds. If you enter 5 characters, and your device has 50 ARC
apps, the UI freezes for about 1.25 seconds.
This CL removes the bottleneck by avoiding the heavy resize operation
when possible.
BUG=673571
TEST=Install 50+ ARC apps to make the issue much more apparent, press
Search to show the text area for app search, enter a few characters
to the area, confirm that the UI is still responsive.
TEST=chrome://tracing (input latency mode) does not show any long
tasks in the UI thread anymore.
Committed: https://crrev.com/e3f59ed9bdc15fc1cccc737e425d40a077c36a32
Cr-Commit-Position: refs/heads/master@{#438238}
Patch Set 1 #
Messages
Total messages: 17 (11 generated)
The CQ bit was checked by yusukes@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== arc: Avoid the resize operation in GetImageForScale() whenever possible ArcAppIcon::Source::GetImageForScale() is one of the lowest level functions for the ARC app icon stuff which can easily be called thousands of times while e.g. the user process a few keys. However, the function does a relatively heavy Skia operation, gfx::ImageSkiaOperations::CreateResizedImage() with RESIZE_BEST, against either IDR_APP_DEFAULT_ICON or IDR_ARC_SUPPORT_ICON_NN, and it turned out that the resize operation actually slowed down Chrome OS. For example, when 50 ARC apps are installed, pressing Search, then 'abcde' on a recent Chromebook blocks the browser's UI thread for (5ms * num_of_arc_apps * num_of_key_presses) which is 1.25 seconds in thie case. This CL removes the bottleneck by avoiding the heavy resize operation when possible. BUG=673571 TEST=Install 50+ ARC apps to make the issue much more apparent, press Search to show the app search ========== to ========== arc: Avoid the resize operation in GetImageForScale() whenever possible ArcAppIcon::Source::GetImageForScale() is one of the lowest level functions for the ARC app icon stuff which can easily be called thousands of times while e.g. the user process a few keys. However, the function does a relatively heavy Skia operation, gfx::ImageSkiaOperations::CreateResizedImage() with RESIZE_BEST, against either IDR_APP_DEFAULT_ICON or IDR_ARC_SUPPORT_ICON_NN, and it turned out that the resize operation actually slowed down Chrome OS. For example, when 50 ARC apps are installed, pressing Search, then 'abcde' on a recent Chromebook blocks the browser's UI thread for (5ms * num_of_arc_apps * num_of_key_presses) which is 1.25 seconds in thie case. This CL removes the bottleneck by avoiding the heavy resize operation when possible. BUG=673571 TEST=Install 50+ ARC apps to make the issue much more apparent, press Search to show the text area for app search, enter a few characters to the area, confirm that the UI is still responsive. TEST=chrome://tracing (input latency mode) does not show any long tasks in the UI thread anymore. ==========
Description was changed from ========== arc: Avoid the resize operation in GetImageForScale() whenever possible ArcAppIcon::Source::GetImageForScale() is one of the lowest level functions for the ARC app icon stuff which can easily be called thousands of times while e.g. the user process a few keys. However, the function does a relatively heavy Skia operation, gfx::ImageSkiaOperations::CreateResizedImage() with RESIZE_BEST, against either IDR_APP_DEFAULT_ICON or IDR_ARC_SUPPORT_ICON_NN, and it turned out that the resize operation actually slowed down Chrome OS. For example, when 50 ARC apps are installed, pressing Search, then 'abcde' on a recent Chromebook blocks the browser's UI thread for (5ms * num_of_arc_apps * num_of_key_presses) which is 1.25 seconds in thie case. This CL removes the bottleneck by avoiding the heavy resize operation when possible. BUG=673571 TEST=Install 50+ ARC apps to make the issue much more apparent, press Search to show the text area for app search, enter a few characters to the area, confirm that the UI is still responsive. TEST=chrome://tracing (input latency mode) does not show any long tasks in the UI thread anymore. ========== to ========== arc: Avoid the resize operation in GetImageForScale() whenever possible to remove UI jank. ArcAppIcon::Source::GetImageForScale() is one of the lowest level functions for the ARC app icon stuff which can easily be called thousands of times while e.g. the user enters a few characters to the app search box. However, the function does a relatively heavy Skia operation, gfx::ImageSkiaOperations::CreateResizedImage() with RESIZE_BEST, against either IDR_APP_DEFAULT_ICON or IDR_ARC_SUPPORT_ICON_NN, and b/33377533 shows that the operation actually slows down Chrome OS UI significantly. For example, on a recent Chromebook, pressing Search, then entering pressing characters to the app search box freezes the browser's UI thread for about (5 * num_of_arc_apps * num_of_key_presses) milliseconds. If you enter 5 characters, and your device has 50 ARC apps, the UI freezes for about 1.25 seconds. This CL removes the bottleneck by avoiding the heavy resize operation when possible. BUG=673571 TEST=Install 50+ ARC apps to make the issue much more apparent, press Search to show the text area for app search, enter a few characters to the area, confirm that the UI is still responsive. TEST=chrome://tracing (input latency mode) does not show any long tasks in the UI thread anymore. ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
yusukes@chromium.org changed reviewers: + khmel@chromium.org, xiyuan@chromium.org
Please take a look.
lgtm
lgtm
The CQ bit was checked by yusukes@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 1, "attempt_start_ts": 1481654096840600, "parent_rev":
"477fdaa297b1bd1a85f11e66046efee5d9fe8f1d", "commit_rev":
"9042c078a4ad69191b0bacf253e78f1e101f42c0"}
Message was sent while issue was closed.
Description was changed from ========== arc: Avoid the resize operation in GetImageForScale() whenever possible to remove UI jank. ArcAppIcon::Source::GetImageForScale() is one of the lowest level functions for the ARC app icon stuff which can easily be called thousands of times while e.g. the user enters a few characters to the app search box. However, the function does a relatively heavy Skia operation, gfx::ImageSkiaOperations::CreateResizedImage() with RESIZE_BEST, against either IDR_APP_DEFAULT_ICON or IDR_ARC_SUPPORT_ICON_NN, and b/33377533 shows that the operation actually slows down Chrome OS UI significantly. For example, on a recent Chromebook, pressing Search, then entering pressing characters to the app search box freezes the browser's UI thread for about (5 * num_of_arc_apps * num_of_key_presses) milliseconds. If you enter 5 characters, and your device has 50 ARC apps, the UI freezes for about 1.25 seconds. This CL removes the bottleneck by avoiding the heavy resize operation when possible. BUG=673571 TEST=Install 50+ ARC apps to make the issue much more apparent, press Search to show the text area for app search, enter a few characters to the area, confirm that the UI is still responsive. TEST=chrome://tracing (input latency mode) does not show any long tasks in the UI thread anymore. ========== to ========== arc: Avoid the resize operation in GetImageForScale() whenever possible to remove UI jank. ArcAppIcon::Source::GetImageForScale() is one of the lowest level functions for the ARC app icon stuff which can easily be called thousands of times while e.g. the user enters a few characters to the app search box. However, the function does a relatively heavy Skia operation, gfx::ImageSkiaOperations::CreateResizedImage() with RESIZE_BEST, against either IDR_APP_DEFAULT_ICON or IDR_ARC_SUPPORT_ICON_NN, and b/33377533 shows that the operation actually slows down Chrome OS UI significantly. For example, on a recent Chromebook, pressing Search, then entering pressing characters to the app search box freezes the browser's UI thread for about (5 * num_of_arc_apps * num_of_key_presses) milliseconds. If you enter 5 characters, and your device has 50 ARC apps, the UI freezes for about 1.25 seconds. This CL removes the bottleneck by avoiding the heavy resize operation when possible. BUG=673571 TEST=Install 50+ ARC apps to make the issue much more apparent, press Search to show the text area for app search, enter a few characters to the area, confirm that the UI is still responsive. TEST=chrome://tracing (input latency mode) does not show any long tasks in the UI thread anymore. Review-Url: https://codereview.chromium.org/2567253002 ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1)
Message was sent while issue was closed.
Description was changed from ========== arc: Avoid the resize operation in GetImageForScale() whenever possible to remove UI jank. ArcAppIcon::Source::GetImageForScale() is one of the lowest level functions for the ARC app icon stuff which can easily be called thousands of times while e.g. the user enters a few characters to the app search box. However, the function does a relatively heavy Skia operation, gfx::ImageSkiaOperations::CreateResizedImage() with RESIZE_BEST, against either IDR_APP_DEFAULT_ICON or IDR_ARC_SUPPORT_ICON_NN, and b/33377533 shows that the operation actually slows down Chrome OS UI significantly. For example, on a recent Chromebook, pressing Search, then entering pressing characters to the app search box freezes the browser's UI thread for about (5 * num_of_arc_apps * num_of_key_presses) milliseconds. If you enter 5 characters, and your device has 50 ARC apps, the UI freezes for about 1.25 seconds. This CL removes the bottleneck by avoiding the heavy resize operation when possible. BUG=673571 TEST=Install 50+ ARC apps to make the issue much more apparent, press Search to show the text area for app search, enter a few characters to the area, confirm that the UI is still responsive. TEST=chrome://tracing (input latency mode) does not show any long tasks in the UI thread anymore. Review-Url: https://codereview.chromium.org/2567253002 ========== to ========== arc: Avoid the resize operation in GetImageForScale() whenever possible to remove UI jank. ArcAppIcon::Source::GetImageForScale() is one of the lowest level functions for the ARC app icon stuff which can easily be called thousands of times while e.g. the user enters a few characters to the app search box. However, the function does a relatively heavy Skia operation, gfx::ImageSkiaOperations::CreateResizedImage() with RESIZE_BEST, against either IDR_APP_DEFAULT_ICON or IDR_ARC_SUPPORT_ICON_NN, and b/33377533 shows that the operation actually slows down Chrome OS UI significantly. For example, on a recent Chromebook, pressing Search, then entering pressing characters to the app search box freezes the browser's UI thread for about (5 * num_of_arc_apps * num_of_key_presses) milliseconds. If you enter 5 characters, and your device has 50 ARC apps, the UI freezes for about 1.25 seconds. This CL removes the bottleneck by avoiding the heavy resize operation when possible. BUG=673571 TEST=Install 50+ ARC apps to make the issue much more apparent, press Search to show the text area for app search, enter a few characters to the area, confirm that the UI is still responsive. TEST=chrome://tracing (input latency mode) does not show any long tasks in the UI thread anymore. Committed: https://crrev.com/e3f59ed9bdc15fc1cccc737e425d40a077c36a32 Cr-Commit-Position: refs/heads/master@{#438238} ==========
Message was sent while issue was closed.
Patchset 1 (id:??) landed as https://crrev.com/e3f59ed9bdc15fc1cccc737e425d40a077c36a32 Cr-Commit-Position: refs/heads/master@{#438238} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
