Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Commit

Permalink
Merge pull request #1197 from adobe/randy/switch-project-perf
Browse files Browse the repository at this point in the history 
switch project performance improvements
  • Loading branch information
@gruehle
gruehle committed Jul 10, 2012
2 parents 77cb5cb + 27b1b49 commit 923b976
Show file tree
Hide file tree
Showing 5 changed files with 148 additions and 83 deletions.
46 changes: 19 additions & 27 deletions src/document/ChangedDocumentTracker.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,37 +47,23 @@ define(function (require, exports, module) {
this._changedPaths = {};
this._windowFocus = true;
this._addListener = this._addListener.bind(this);
this._removeListener = this._removeListener.bind(this);
this._onChange = this._onChange.bind(this);
this._onWindowFocus = this._onWindowFocus.bind(this);

$(DocumentManager).on("workingSetAdd", function (event, fileEntry) {
// Only track documents in the current project
if (ProjectManager.isWithinProject(fileEntry.fullPath)) {
DocumentManager.getDocumentForPath(fileEntry.fullPath).done(function (doc) {
self._addListener(doc);
});
}
});

$(DocumentManager).on("workingSetRemove", function (event, fileEntry) {

$(DocumentManager).on("afterDocumentCreate", function (event, doc) {
// Only track documents in the current project
if (ProjectManager.isWithinProject(fileEntry.fullPath)) {
DocumentManager.getDocumentForPath(fileEntry.fullPath).done(function (doc) {
$(doc).off("change", self._onChange);
doc.releaseRef();
});
if (ProjectManager.isWithinProject(doc.file.fullPath)) {
self._addListener(doc);
}
});

// DocumentManager has already initialized the working set
DocumentManager.getWorkingSet().forEach(function (fileEntry) {
// Can't use getOpenDocumentForPath() here since being in the working set
// does not guarantee that the file is opened (e.g. at startup)
DocumentManager.getDocumentForPath(fileEntry.fullPath).done(function (doc) {
self._addListener(doc);
});

$(DocumentManager).on("beforeDocumentDelete", function (event, doc) {
// In case a document somehow remains loaded after its project
// has been closed, unconditionally attempt to remove the listener.
self._removeListener(doc);
});

$(window).focus(this._onWindowFocus);
}

Expand All @@ -87,9 +73,15 @@ define(function (require, exports, module) {
*/
ChangedDocumentTracker.prototype._addListener = function (doc) {
$(doc).on("change", this._onChange);
doc.addRef();
};


/**
* @private
*/
ChangedDocumentTracker.prototype._removeListener = function (doc) {
$(doc).off("change", this._onChange);
};

/**
* @private
* Assumes all files are changed when the window loses and regains focus.
Expand Down
12 changes: 9 additions & 3 deletions src/document/DocumentCommandHandlers.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,8 @@ define(function (require, exports, module) {
})
.fail(function (fileError) {
FileUtils.showFileOpenError(fileError.code, fullPath).done(function () {
// For performance, we do lazy checking of file existence, so it may be in working set
DocumentManager.removeFromWorkingSet(new NativeFileSystem.FileEntry(fullPath));
EditorManager.focusEditor();
result.reject();
});
Expand Down Expand Up @@ -189,9 +191,13 @@ define(function (require, exports, module) {
var i;

if (paths.length > 0) {
for (i = 0; i < paths.length - 1; i++) {
DocumentManager.addToWorkingSet(new NativeFileSystem.FileEntry(paths[i]));
}
// Add all files to the working set without verifying that
// they still exist on disk (for faster opening)
var filesToOpen = [];
paths.forEach(function (file) {
filesToOpen.push(new NativeFileSystem.FileEntry(file));
});
DocumentManager.addListToWorkingSet(filesToOpen);

doOpen(paths[paths.length - 1])
.done(function (doc) {
Expand Down
129 changes: 79 additions & 50 deletions src/document/DocumentManager.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,12 @@
* - currentDocumentChange -- When the value of getCurrentDocument() changes.
* - workingSetAdd -- When a file is added to the working set (see getWorkingSet()). The 2nd arg
* to the listener is the added FileEntry.
* - workingSetAddList -- When a list of files are added to the working set (e.g. project open, multiple file open).
* The 2nd arg to the listener is the array of added FileEntry objects.
* - workingSetRemove -- When a file is removed from the working set (see getWorkingSet()). The
* 2nd arg to the listener is the removed FileEntry.
* - workingSetRemoveList -- When a list of files is to be removed from the working set (e.g. project close).
* The 2nd arg to the listener is the array of removed FileEntry objects.
*
* These are jQuery events, so to listen for them you do something like this:
* $(DocumentManager).on("eventname", handler);
Expand Down Expand Up @@ -216,13 +220,45 @@ define(function (require, exports, module) {
// Dispatch event
$(exports).triggerHandler("workingSetAdd", file);
}


/**
* Adds the given file list to the end of the working set list.
* Does not change which document is currently open in the editor.
* More efficient than calling addToWorkingSet() (in a loop) for
* a list of files because there's only 1 redraw at the end
* @param {!FileEntryArray} fileList
*/
function addListToWorkingSet(fileList) {
var uniqueFileList = [];

// Process only files not already in working set
fileList.forEach(function (file) {
// If doc is already in working set, don't add it again
if (findInWorkingSet(file.fullPath) === -1) {
uniqueFileList.push(file);

// Add
_workingSet.push(file);

// Add to MRU order: either first or last, depending on whether it's already the current doc or not
if (_currentDocument && _currentDocument.file.fullPath === file.fullPath) {
_workingSetMRUOrder.unshift(file);
} else {
_workingSetMRUOrder.push(file);
}
}
});

// Dispatch event
$(exports).triggerHandler("workingSetAddList", [uniqueFileList]);
}

/**
* Removes the given file from the working set list, if it was in the list. Does not change
* the current editor even if it's for this file.
* @param {!FileEntry} file
*/
function _removeFromWorkingSet(file) {
function removeFromWorkingSet(file) {
// If doc isn't in working set, do nothing
var index = findInWorkingSet(file.fullPath);
if (index === -1) {
Expand All @@ -236,7 +272,21 @@ define(function (require, exports, module) {
// Dispatch event
$(exports).triggerHandler("workingSetRemove", file);
}


/**
* Removes all files from the working set list.
*/
function _removeAllFromWorkingSet() {
var fileList = _workingSet;

// Remove all
_workingSet = [];
_workingSetMRUOrder = [];

// Dispatch event
$(exports).triggerHandler("workingSetRemoveList", [fileList]);
}

/**
* Moves document to the front of the MRU list, IF it's in the working set; no-op otherwise.
* @param {!Document}
Expand Down Expand Up @@ -370,10 +420,10 @@ define(function (require, exports, module) {

// Remove closed doc from working set, if it was in there
// This happens regardless of whether the document being closed was the current one or not
_removeFromWorkingSet(file);
removeFromWorkingSet(file);

// Note: EditorManager will dispose the closed document's now-unneeded editor either in
// response to the editor-swap call above, or the _removeFromWorkingSet() call, depending on
// response to the editor-swap call above, or the removeFromWorkingSet() call, depending on
// circumstances. See notes in EditorManager for more.
}

Expand All @@ -383,9 +433,7 @@ define(function (require, exports, module) {
*/
function closeAll() {
_clearCurrentDocument();

var wsClone = _workingSet.slice(0); // can't delete from the same array we're iterating
wsClone.forEach(_removeFromWorkingSet);
_removeAllFromWorkingSet();
}


Expand Down Expand Up @@ -516,14 +564,16 @@ define(function (require, exports, module) {
if (_openDocuments[this.file.fullPath]) {
throw new Error("Document for this path already in _openDocuments!");
}

_openDocuments[this.file.fullPath] = this;
$(exports).triggerHandler("afterDocumentCreate", this);
}
this._refCount++;
};
/** Remove a ref that was keeping this Document alive */
Document.prototype.releaseRef = function () {
//console.log("---REF--- "+this);

this._refCount--;
if (this._refCount < 0) {
throw new Error("Document ref count has fallen below zero!");
Expand All @@ -533,6 +583,8 @@ define(function (require, exports, module) {
if (!_openDocuments[this.file.fullPath]) {
throw new Error("Document with references was not in _openDocuments!");
}

$(exports).triggerHandler("beforeDocumentDelete", this);
delete _openDocuments[this.file.fullPath];
}
};
Expand Down Expand Up @@ -950,49 +1002,24 @@ define(function (require, exports, module) {
var filesToOpen = [],
activeFile;

// in parallel, check if files exist
// TODO: (issue #298) delay this check until it becomes the current document?
function checkOneFile(value, index) {
var oneFileResult = new $.Deferred();

// check if the file still exists; silently drop from working set if it doesn't
projectRoot.getFile(value.file, {},
function (fileEntry) {
// maintain original sequence
filesToOpen[index] = fileEntry;
// Add all files to the working set without verifying that
// they still exist on disk (for faster project switching)
files.forEach(function (value, index) {
filesToOpen.push(new NativeFileSystem.FileEntry(value.file));
if (value.active) {
activeFile = value.file;
}
});
addListToWorkingSet(filesToOpen);

if (value.active) {
activeFile = fileEntry;
}
oneFileResult.resolve();
},
function (error) {
filesToOpen[index] = null;
oneFileResult.resolve();
});

return oneFileResult.promise();
// Initialize the active editor
if (!activeFile && _workingSet.length > 0) {
activeFile = _workingSet[0].fullPath;
}

var result = Async.doInParallel(files, checkOneFile, false);

result.done(function () {
// Add all existing files to the working set
filesToOpen.forEach(function (file, index) {
if (file) {
addToWorkingSet(file);
}
});

// Initialize the active editor
if (!activeFile && _workingSet.length > 0) {
activeFile = _workingSet[0];
}

if (activeFile) {
CommandManager.execute(Commands.FILE_OPEN, { fullPath: activeFile.fullPath });
}
});
if (activeFile) {
CommandManager.execute(Commands.FILE_OPEN, { fullPath: activeFile });
}
}


Expand All @@ -1006,6 +1033,8 @@ define(function (require, exports, module) {
exports.getAllOpenDocuments = getAllOpenDocuments;
exports.setCurrentDocument = setCurrentDocument;
exports.addToWorkingSet = addToWorkingSet;
exports.addListToWorkingSet = addListToWorkingSet;
exports.removeFromWorkingSet = removeFromWorkingSet;
exports.getNextPrevFile = getNextPrevFile;
exports.beginDocumentNavigation = beginDocumentNavigation;
exports.finalizeDocumentNavigation = finalizeDocumentNavigation;
Expand All @@ -1015,7 +1044,7 @@ define(function (require, exports, module) {

// Setup preferences
_prefs = PreferencesManager.getPreferenceStorage(PREFERENCES_CLIENT_ID);
$(exports).bind("currentDocumentChange workingSetAdd workingSetRemove", _savePreferences);
$(exports).bind("currentDocumentChange workingSetAdd workingSetAddList workingSetRemove workingSetRemoveList", _savePreferences);

// Performance measurements
PerfUtils.createPerfMeasurement("DOCUMENT_MANAGER_GET_DOCUMENT_FOR_PATH", "DocumentManager.getDocumentForPath()");
Expand Down
9 changes: 9 additions & 0 deletions src/editor/EditorManager.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -430,6 +430,13 @@ define(function (require, exports, module) {
}
// else, file was listed in working set but never shown in the editor - ignore
}

function _onWorkingSetRemoveList(event, removedFiles) {
removedFiles.forEach(function (removedFile) {
_onWorkingSetRemove(event, removedFile);
});
}

// Note: there are several paths that can lead to an editor getting destroyed
// - file was in working set, but not in current editor; then closed (via working set "X" button)
// --> handled by _onWorkingSetRemove()
Expand Down Expand Up @@ -546,6 +553,8 @@ define(function (require, exports, module) {
// Initialize: register listeners
$(DocumentManager).on("currentDocumentChange", _onCurrentDocumentChange);
$(DocumentManager).on("workingSetRemove", _onWorkingSetRemove);
$(DocumentManager).on("workingSetRemoveList", _onWorkingSetRemoveList);

// Add this as a capture handler so we're guaranteed to run it before the editor does its own
// refresh on resize.
window.addEventListener("resize", _updateEditorSize, true);
Expand Down
35 changes: 32 additions & 3 deletions src/project/WorkingSetView.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,17 @@ define(function (require, exports, module) {
_createNewListItem(file);
_redraw();
}


/**
* @private
*/
function _handleFileListAdded(files) {
files.forEach(function (file) {
_createNewListItem(file);
});
_redraw();
}

/**
* @private
*/
Expand All @@ -291,6 +301,17 @@ define(function (require, exports, module) {
_redraw();
}

function _handleRemoveList(removedFiles) {
removedFiles.forEach(function (file) {
var $listItem = _findListItemFromFile(file);
if ($listItem) {
$listItem.remove();
}
});

_redraw();
}

/**
* @private
* @param {Document} doc
Expand All @@ -313,11 +334,19 @@ define(function (require, exports, module) {
$(DocumentManager).on("workingSetAdd", function (event, addedFile) {
_handleFileAdded(addedFile);
});


$(DocumentManager).on("workingSetAddList", function (event, addedFiles) {
_handleFileListAdded(addedFiles);
});

$(DocumentManager).on("workingSetRemove", function (event, removedFile) {
_handleFileRemoved(removedFile);
});


$(DocumentManager).on("workingSetRemoveList", function (event, removedFiles) {
_handleRemoveList(removedFiles);
});

$(DocumentManager).on("dirtyFlagChange", function (event, doc) {
_handleDirtyFlagChanged(doc);
});
Expand Down

0 comments on commit 923b976

Please sign in to comment.