Keyboard shortcut viewer: update is costly |
||||
Issue descriptionThe keyboard-shortcut-viewer app allows doing a search. Attached is a trace for entering the 'n' key in the search box. Look at the 'Service: shortcut_viewer_app' process, at around 2,800.211 timestamp. The task being executed there is KeyboardShortcutView::ShowSearchResults() [1]. It takes over 1 second to complete (~1150ms). Interesting observations: . RenderTextHarfBuzz:EnsureLayoutRunList() is called over 6000 times. . View::Layout(bounds_changed) is called over 2000 times. . The following paint of the view tree takes ~125+ms. Similar to issue 865286 , I think views and text layout in ui can be optimized to make this better. [1] https://cs.chromium.org/chromium/src/ash/components/shortcut_viewer/views/keyboard_shortcut_view.cc?l=359
,
Jul 19
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/384306f75ac2698898541dc17d32aa0486bd47d1 commit 384306f75ac2698898541dc17d32aa0486bd47d1 Author: Sadrul Habib Chowdhury <sadrul@chromium.org> Date: Thu Jul 19 18:03:02 2018 ksv: Change when the startup-time is reported. Instead of reporting the time to request the Widget to be visible, report the time when the widget actually shows up on screen. So this includes the time spent on, for example, View::Paint() etc. BUG=865295 Change-Id: I040878e6f897b066102608e32ba3cd6d353b01d0 Reviewed-on: https://chromium-review.googlesource.com/1143603 Reviewed-by: Tao Wu <wutao@chromium.org> Commit-Queue: Sadrul Chowdhury <sadrul@chromium.org> Cr-Commit-Position: refs/heads/master@{#576558} [modify] https://crrev.com/384306f75ac2698898541dc17d32aa0486bd47d1/ash/components/shortcut_viewer/views/keyboard_shortcut_view.cc [modify] https://crrev.com/384306f75ac2698898541dc17d32aa0486bd47d1/ash/components/shortcut_viewer/views/keyboard_shortcut_view_unittest.cc
,
Jul 19
Hi sadrul@, which device is it. For the search result, we do relayout the discription StyledLabel, because we need to hightlight/bold the matched query. ps. There is also additional 250ms debounce delay [1] [1] https://cs.chromium.org/chromium/src/ash/components/shortcut_viewer/views/keyboard_shortcut_view.cc?l=223&rcl=1ce671509c922ab463f3c2c9aff736745ade2850
,
Jul 19
The time mentioned in OP does not include the debounce-time.
This was on an original pixel-chromebook ('link' board). I have requested for a newer device, since 'link' I believe has been eol'ed.
I will be mildly surprised if the performance is much better on newer devices though.
,
Jul 26
Request to merge cl in #2 to M69, so that we can see improvement.
,
Jul 26
To clarify: the CL in #2 does not make any change that would affect the user-experience. It just includes additional steps in the compositor pipeline so the reported number better reflects what the user experiences.
,
Jul 26
Remove the request. The cl in #2 actually landed in 69.0.3497.0. Thanks sadrul@!
,
Aug 25
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1c9af66df3f0b677961d6f7149e541ab6c83045a commit 1c9af66df3f0b677961d6f7149e541ab6c83045a Author: Sadrul Habib Chowdhury <sadrul@chromium.org> Date: Sat Aug 25 02:36:50 2018 ksv: Measure how long it takes to update for search result. Report how long it takes to update the views hierarchy in response to a search-string. Also report how long it takes before the update is visible on screen. BUG=865295 Change-Id: I103aa5cbd86be3189ab44121e72d375e6d742b4b Reviewed-on: https://chromium-review.googlesource.com/1188755 Reviewed-by: Tao Wu <wutao@chromium.org> Reviewed-by: Steven Holte <holte@chromium.org> Commit-Queue: Sadrul Chowdhury <sadrul@chromium.org> Cr-Commit-Position: refs/heads/master@{#586111} [modify] https://crrev.com/1c9af66df3f0b677961d6f7149e541ab6c83045a/ash/components/shortcut_viewer/views/keyboard_shortcut_view.cc [modify] https://crrev.com/1c9af66df3f0b677961d6f7149e541ab6c83045a/tools/metrics/histograms/histograms.xml
,
Sep 17
|
||||
►
Sign in to add a comment |
||||
Comment 1 by sadrul@chromium.org
, Jul 19