Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(528)

Issue 9580035: Revert 124674 - Improving file manager js/css performance (Closed)

Created:
8 years, 9 months ago by zel
Modified:
8 years, 9 months ago
Reviewers:
Rick Byers
CC:
chromium-reviews, rginda+watch_chromium.org, arv (Not doing code reviews), achuith+watch_chromium.org
Visibility:
Public.

Description

Revert 124674 - Improving file manager js/css performance This change attempts to reduce the load time of the file manager (especially on slow Alex devices) by trimming unnecessary work done in javascript and reducing layouts. - Enable batch updating in cr.ui.Table (exactly how it's done in cr.ui.List). - Add more 'on complete' callbacks to some of the FileManager infrastructure so we know when to stop batch UI updates. - Use batch updates for some operations which profiling indicates causes non-trivial amounts of duplicated work. In particular, in my testing this reduces the number of (sometimes expensive) List.redraw() calls on startup for the table from 6 down to 1, and for the roots list from 4 down to 2. Measurements on alex are quite variable, but these changes result in about 70ms savings on startup (or about 17% of the time spent under 'v8.callChromeHiddenMethod' - i.e. JS callbacks through the extension system, which itself is about 1/3rd of total load time). The majority of file manager load time is spent inside of v8, and there are many more opportunities like these to trim various code paths. But it seems clear that major improvements are going to require more drastic approaches (eg. I'm experimenting with painting the initial UI after parsing/running a small fraction of the JS). BUG=105181 TEST= Review URL: http://codereview.chromium.org/9379023 TBR=rbyers@chromium.org Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=124786

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+27 lines, -93 lines) Patch
MM chrome/browser/resources/file_manager/js/directory_model.js View 6 chunks +12 lines, -29 lines 0 comments Download
M chrome/browser/resources/file_manager/js/file_manager.js View 7 chunks +15 lines, -39 lines 0 comments Download
M chrome/browser/resources/shared/js/cr/ui/table.js View 1 chunk +0 lines, -10 lines 0 comments Download
M chrome/browser/resources/shared/js/cr/ui/table/table_header.js View 1 chunk +0 lines, -15 lines 0 comments Download

Messages

Total messages: 2 (0 generated)
zel
8 years, 9 months ago (2012-03-03 00:30:38 UTC) #1
Rick Byers
8 years, 9 months ago (2012-03-03 03:28:47 UTC) #2
LGTM
Sorry about that - I didn't notice the gdata UI work had landed a couple days
ago.

Powered by Google App Engine
This is Rietveld 408576698