New issue
Advanced search Search tips

Issue 865286 link

Starred by 1 user

Issue metadata

Status: Duplicate
Merged: issue 849413
Owner: ----
Closed: Jul 19
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

Keyboard shortcut viewer: startup is costly

Project Member Reported by sadrul@chromium.org, Jul 19

Issue description

Attached is a trace for starting up the keyboard-shortcut-viewer. Look specifically at the 'Service: shortcut_viewer_app' process. The initial views::Widget set up (in Widget::Init()) takes ~490ms, and the following first paint takes ~25ms, followed by another ~50ms of raster. Some interesting observations:
 . RenderTextHarfBuzz:EnsureLayoutRunList() gets called ~3000 times during Widget::Init().
 . View::Layout(bounds_changed) gets called ~2400 times.
 . RenderTextHarfBuzz:EnsureLayoutRunList() is called twice before Widget::Init(), each of which takes 20+ms (I assume during KeyboardShortcutView::InitViews()).

This seems unnaturally expensive. We should probably look into optimizing the views layout code. I am not familiar with RenderTextHarfBuzz or other parts of text-layout code, but perhaps something could be improved there too?

cc'ing some chromeos UI folks, other people who are working on related issues (e.g.  issue 835983 , issue 858944 etc.)
 
trace_ksv-create.json.gz
866 KB Download
Cc: ccameron@chromium.org
Re: RenderTextHarfBuzz, also see issue 862773. The CoreText parts of that aren't cross platform, but it definitely seems like we could be calling it less overall as in https://chromium-review.googlesource.com/c/chromium/src/+/1139891.

In this specific case, I think it's likely that KeyboardShortcutView has child views that use RenderText directly without doing aggressive caching of calculated size, as in views::Label or what we recently added to Omnibox text views.
I believe an easy fix that was talked about it to only create views for the things that are visible.
Mergedinto: 849413
Status: Duplicate (was: Untriaged)
 lgrey@: KeyboardShortcutView is using StyledLabel, which will create many children Labels.

Sign in to add a comment