New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 754780 link

Starred by 1 user

Issue metadata

Status: Archived
Owner:
Closed: Aug 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Task



Sign in to add a comment

feature flag for IsFullscreenAppListEnabled calls 870 times on startup. Cache more.

Project Member Reported by newcomer@chromium.org, Aug 11 2017

Issue description

Reduce calls to features::IsFullscreenAppListEnabled.

On start up the flag is called 870 times, this is too much.

The vast majority of calls are coming from AppsGridView. We can probably save a lot of time by caching this flag.
 
4,711 calls are made upon a search of "cat" in the new launcher. 

Time to call the feature flag is measured at "0" in UMA, so maybe this isn't worth worry about.
Further investigation:
Each call takes on average 1.6 microseconds to show, we do it about 870 times per show. 
On each show action we take 1.36 milliseconds just to call this function.
From what I know we are supposed to try to limit chunks on the UI thread to 5ms. Should we worry about this still?
Project Member

Comment 3 by bugdroid1@chromium.org, Aug 22 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/af2534308dced33cb48ad165619e590d053f8288

commit af2534308dced33cb48ad165619e590d053f8288
Author: Alex Newcomer <newcomer@chromium.org>
Date: Tue Aug 22 20:07:03 2017

cros: cached an inneficient method in AppsGridView.

This cache reduces calls to IsFullscreenAppListEnabled by 50%.
It also means we will create 1112 less gfx::Rects and 1112 less

gfx: :Size objects. and we will call gfx::Rect.offset 1112 less times.
Bug:  754780 
Change-Id: I3afbece216932a12fdfbbff644235af8664ff20c
Reviewed-on: https://chromium-review.googlesource.com/620110
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Alex Newcomer <newcomer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#496416}
[modify] https://crrev.com/af2534308dced33cb48ad165619e590d053f8288/ui/app_list/views/apps_grid_view.cc

Status: Fixed (was: Untriaged)
Project Member

Comment 5 by bugdroid1@chromium.org, Aug 25 2017

Labels: merge-merged-3163
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/49dd9ed8351548ea34d49f4bfd4af89244b1ec39

commit 49dd9ed8351548ea34d49f4bfd4af89244b1ec39
Author: Maajid <maajid@chromium.org>
Date: Fri Aug 25 19:13:03 2017

cros: cached an inneficient method in AppsGridView.

This cache reduces calls to IsFullscreenAppListEnabled by 50%.
It also means we will create 1112 less gfx::Rects and 1112 less

TBR=newcomer@chromium.org

(cherry picked from commit af2534308dced33cb48ad165619e590d053f8288)

gfx: :Size objects. and we will call gfx::Rect.offset 1112 less times.
Bug:  754780 
Change-Id: I3afbece216932a12fdfbbff644235af8664ff20c
Reviewed-on: https://chromium-review.googlesource.com/620110
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Alex Newcomer <newcomer@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#496416}
Reviewed-on: https://chromium-review.googlesource.com/635949
Reviewed-by: Vadim Tryshev <vadimt@chromium.org>
Cr-Commit-Position: refs/branch-heads/3163@{#896}
Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528}
[modify] https://crrev.com/49dd9ed8351548ea34d49f4bfd4af89244b1ec39/ui/app_list/views/apps_grid_view.cc

Comment 6 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)

Sign in to add a comment