Commit 6b1af1fe authored by zelidrag@chromium.org's avatar zelidrag@chromium.org

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
Review URL: https://chromiumcodereview.appspot.com/9580035

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@124786 0039d316-1c4b-4281-b951-d872f2087c98
parent 57a48d36
......@@ -522,26 +522,10 @@ DirectoryModel.prototype = {
this.dispatchEvent(e);
},
/**
* Change the state of the model to reflect the specified path (either a
* file or directory).
*
* @param {string} path The root path to use
* @param {Function=} opt_loadedCallback Invoked when the entire directory
* has been loaded and any default file selected.
* @param {Function=} opt_pathResolveCallback Invoked as soon as the path has
* been resolved, and called with the base and leaf portions of the path
* name, and a flag indicating if the entry exists.
*/
setupPath: function(path, opt_loadedCallback, opt_pathResolveCallback) {
setupPath: function(path, opt_pathResolveCallback, opt_fileSelectCallback) {
// Split the dirname from the basename.
var ary = path.match(/^(?:(.*)\/)?([^\/]*)$/);
var autoSelect = function() {
this.selectIndex(this.autoSelectIndex_);
if (opt_loadedCallback)
opt_loadedCallback();
}.bind(this);
var autoSelect = this.selectIndex.bind(this, this.autoSelectIndex_);
if (!ary) {
console.warn('Unable to split default path: ' + path);
this.changeDirectoryEntry_(this.root_, autoSelect, true);
......@@ -570,8 +554,8 @@ DirectoryModel.prototype = {
this.changeDirectoryEntry_(baseDirEntry,
function() {
this.selectEntry(leafEntry.name);
if (opt_loadedCallback)
opt_loadedCallback();
if (opt_fileSelectCallback)
opt_fileSelectCallback();
}.bind(this),
false /*HACK*/);
// TODO(kaznacheev): Fix history.replaceState for the File Browser and
......@@ -593,11 +577,11 @@ DirectoryModel.prototype = {
baseName + '": ' + err);
if (path != '/' + DirectoryModel.DOWNLOADS_DIRECTORY) {
// Can't find the provided path, let's go to default one instead.
this.setupDefaultPath(opt_loadedCallback);
this.setupDefaultPath();
} else {
// Well, we can't find the downloads dir. Let's just show something,
// or we will get an infinite recursion.
this.changeDirectory('/', opt_loadedCallback, true);
this.changeDirectory('/', undefined, true);
}
}.bind(this);
......@@ -626,10 +610,8 @@ DirectoryModel.prototype = {
}
},
setupDefaultPath: function(opt_callback) {
this.getDefaultDirectory_(function(path) {
this.setupPath(path, opt_callback);
}.bind(this));
setupDefaultPath: function() {
this.getDefaultDirectory_(this.setupPath.bind(this));
},
getDefaultDirectory_: function(callback) {
......@@ -774,7 +756,7 @@ DirectoryModel.prototype = {
onGData, onGDataError);
},
updateRoots: function(opt_callback) {
updateRoots: function(opt_changeDirectoryTo) {
var self = this;
this.resolveRoots_(function(rootEntries) {
var dm = self.rootsList_;
......@@ -782,8 +764,9 @@ DirectoryModel.prototype = {
dm.splice.apply(dm, args);
self.updateRootsListSelection_();
if (opt_callback)
opt_callback();
if (opt_changeDirectoryTo)
self.changeDirectory(opt_changeDirectoryTo);
});
},
......
......@@ -477,7 +477,7 @@ FileManager.prototype = {
(this.dialogType_ == FileManager.DialogType.FULL_PAGE ||
this.dialogType_ == FileManager.DialogType.SELECT_OPEN_MULTI_FILE);
this.table_.startBatchUpdates();
this.table_.list.startBatchUpdates();
this.grid_.startBatchUpdates();
this.initFileList_();
......@@ -545,7 +545,7 @@ FileManager.prototype = {
this.createMetadataProvider_();
this.table_.endBatchUpdates();
this.table_.list.endBatchUpdates();
this.grid_.endBatchUpdates();
metrics.recordInterval('Load.DOM');
......@@ -756,9 +756,7 @@ FileManager.prototype = {
// TODO(dgozman): add "Add a drive" item.
this.rootsList_.dataModel = this.directoryModel_.rootsList;
this.directoryModel_.updateRoots(function() {
self.rootsList_.endBatchUpdates();
});
this.directoryModel_.updateRoots();
};
FileManager.prototype.initGData_ = function() {
......@@ -1397,14 +1395,6 @@ FileManager.prototype = {
* window has neither.
*/
FileManager.prototype.setupCurrentDirectory_ = function() {
// Avoid a bunch of intermediate list redraws while the data model is
// cleared and updated. Note that it may (or may not) be desirable to draw
// partial results as we get them, but today the DirectoryReader API
// generally returns all entries in one chunk so even without this batching
// we wouldn't have incremental updates.
this.table_.startBatchUpdates();
var onLoaded = this.table_.endBatchUpdates.bind(this.table_);
if (location.hash) {
// Location hash has the highest priority.
var path = decodeURI(location.hash.substr(1));
......@@ -1420,15 +1410,10 @@ FileManager.prototype = {
this.document_.body.appendChild(shade);
function removeShade() { shade.parentNode.removeChild(shade) }
// Keep track of whether the path is identified as an existing leaf
// node. Note that onResolve is guaranteed to be called (exactly once)
// before onLoadedActivateLeaf.
var foundLeaf = true;
function onResolve(baseName, leafName, exists) {
if (!exists || leafName == '') {
// Non-existent file or a directory. Remove the shade immediately.
removeShade();
foundLeaf = false;
}
}
......@@ -1436,38 +1421,33 @@ FileManager.prototype = {
// of urls instead of a selection. This will remove the need to wait
// until the selection is done.
var self = this;
function onLoadedActivateLeaf() {
onLoaded();
if (foundLeaf) {
self.dispatchDefaultTask_();
setTimeout(removeShade, 1000);
}
function onFileSelected() {
self.dispatchDefaultTask_();
setTimeout(removeShade, 1000);
}
this.directoryModel_.setupPath(path, onLoadedActivateLeaf, onResolve);
this.directoryModel_.setupPath(path, onResolve, onFileSelected);
return;
}
this.directoryModel_.setupPath(path, onLoaded);
this.directoryModel_.setupPath(path);
return;
}
if (this.params_.defaultPath) {
var path = this.params_.defaultPath;
if (this.dialogType_ == FileManager.DialogType.SELECT_SAVEAS_FILE) {
this.directoryModel_.setupPath(path, onLoaded,
function(basePath, leafName) {
this.filenameInput_.value = leafName;
this.selectDefaultPathInFilenameInput_();
}.bind(this));
this.directoryModel_.setupPath(path, function(basePath, leafName) {
this.filenameInput_.value = leafName;
this.selectDefaultPathInFilenameInput_();
}.bind(this));
return;
}
this.directoryModel_.setupPath(path, onLoaded);
this.directoryModel_.setupPath(path);
return;
}
this.directoryModel_.setupDefaultPath(onLoaded);
this.directoryModel_.setupDefaultPath();
};
/**
......@@ -2420,11 +2400,7 @@ FileManager.prototype = {
// Even if something failed root list should be rescanned.
// Failed mounts can "give" us new devices which might be formatted,
// so we have to refresh root list then.
self.directoryModel_.updateRoots(function() {
if (changeDirectoryTo) {
self.directoryModel_.changeDirectory(changeDirectoryTo);
}
});
self.directoryModel_.updateRoots(changeDirectoryTo);
});
};
......
......@@ -204,16 +204,6 @@ cr.define('cr.ui', function() {
this.header_.redraw();
},
startBatchUpdates: function() {
this.list_.startBatchUpdates();
this.header_.startBatchUpdates();
},
endBatchUpdates: function() {
this.list_.endBatchUpdates();
this.header_.endBatchUpdates();
},
/**
* Resize the table columns.
*/
......
......@@ -68,25 +68,10 @@ cr.define('cr.ui.table', function() {
}
},
batchCount_: 0,
startBatchUpdates: function() {
this.batchCount_++;
},
endBatchUpdates: function() {
this.batchCount_--;
if (this.batchCount_ == 0)
this.redraw();
},
/**
* Redraws table header.
*/
redraw: function() {
if (this.batchCount_ != 0)
return;
var cm = this.table_.columnModel;
var dm = this.table_.dataModel;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment