From 94ed8b3d42707a652cb3b6279deb73aceadc7570 Mon Sep 17 00:00:00 2001 From: Martin Zagora Date: Fri, 18 Nov 2016 03:38:02 +1100 Subject: [PATCH] improvements to file system events processing for search (#12885) * improvements to file system events processing * working on comments * jsdoc * little bit of optimization --- src/search/FindInFiles.js | 198 ++++++++++++++++++++++++++------------ 1 file changed, 137 insertions(+), 61 deletions(-) diff --git a/src/search/FindInFiles.js b/src/search/FindInFiles.js index 9e416b8fe17..02954dd3bae 100644 --- a/src/search/FindInFiles.js +++ b/src/search/FindInFiles.js @@ -73,12 +73,24 @@ define(function (require, exports, module) { var searchModel = new SearchModel(); /* Forward declarations */ - var _documentChangeHandler, _fileSystemChangeHandler, _fileNameChangeHandler, clearSearch; + var _documentChangeHandler, + _fileSystemChangeHandler, + _processCachedFileSystemEvents, + _debouncedFileSystemChangeHandler, + _fileNameChangeHandler, + clearSearch; + + /** + * Waits for FS changes to stack up until processing them + * (scripts like npm install can do a lot of movements on the disk) + * @const + */ + var FILE_SYSTEM_EVENT_DEBOUNCE_TIME = 100; /** Remove the listeners that were tracking potential search result changes */ function _removeListeners() { DocumentModule.off("documentChange", _documentChangeHandler); - FileSystem.off("change", _fileSystemChangeHandler); + FileSystem.off("change", _debouncedFileSystemChangeHandler); DocumentManager.off("fileNameChange", _fileNameChangeHandler); } @@ -88,7 +100,7 @@ define(function (require, exports, module) { _removeListeners(); DocumentModule.on("documentChange", _documentChangeHandler); - FileSystem.on("change", _fileSystemChangeHandler); + FileSystem.on("change", _debouncedFileSystemChangeHandler); DocumentManager.on("fileNameChange", _fileNameChangeHandler); } @@ -658,7 +670,7 @@ define(function (require, exports, module) { * @param {array} fileList The list of files that changed. */ function filesChanged(fileList) { - if (FindUtils.isNodeSearchDisabled() || fileList.length === 0) { + if (FindUtils.isNodeSearchDisabled() || !fileList || fileList.length === 0) { return; } var updateObject = { @@ -676,7 +688,7 @@ define(function (require, exports, module) { * @param {array} fileList The list of files that was removed. */ function filesRemoved(fileList) { - if (FindUtils.isNodeSearchDisabled()) { + if (FindUtils.isNodeSearchDisabled() || !fileList || fileList.length === 0) { return; } var updateObject = { @@ -732,69 +744,83 @@ define(function (require, exports, module) { /* * Remove existing search results that match the given entry's path - * @param {(File|Directory)} entry + * @param {Array.<(File|Directory)>} entries */ - function _removeSearchResultsForEntry(entry) { - Object.keys(searchModel.results).forEach(function (fullPath) { - if (fullPath === entry.fullPath || - (entry.isDirectory && fullPath.indexOf(entry.fullPath) === 0)) { - // node search : inform node that the file is removed - filesRemoved([fullPath]); - if (findOrReplaceInProgress) { - searchModel.removeResults(fullPath); - resultsChanged = true; + function _removeSearchResultsForEntries(entries) { + var fullPaths = []; + entries.forEach(function (entry) { + Object.keys(searchModel.results).forEach(function (fullPath) { + if (fullPath === entry.fullPath || + (entry.isDirectory && fullPath.indexOf(entry.fullPath) === 0)) { + // node search : inform node that the file is removed + fullPaths.push(fullPath); + if (findOrReplaceInProgress) { + searchModel.removeResults(fullPath); + resultsChanged = true; + } } - } + }); }); + // this should be called once with a large array instead of numerous calls with single items + filesRemoved(fullPaths); } /* - * Add new search results for this entry and all of its children - * @param {(File|Directory)} entry + * Add new search results for these entries and all of its children + * @param {Array.<(File|Directory)>} entries * @return {jQuery.Promise} Resolves when the results have been added */ - function _addSearchResultsForEntry(entry) { - var addedFiles = [], - addedFilePaths = [], - deferred = new $.Deferred(); - - // gather up added files - var visitor = function (child) { - // Replicate filtering that getAllFiles() does - if (ProjectManager.shouldShow(child)) { - if (child.isFile && _isReadableText(child.name)) { - // Re-check the filtering that the initial search applied - if (_inSearchScope(child)) { - addedFiles.push(child); - addedFilePaths.push(child.fullPath); + function _addSearchResultsForEntries(entries) { + var fullPaths = []; + return Async.doInParallel(entries, function (entry) { + var addedFiles = [], + addedFilePaths = [], + deferred = new $.Deferred(); + + // gather up added files + var visitor = function (child) { + // Replicate filtering that getAllFiles() does + if (ProjectManager.shouldShow(child)) { + if (child.isFile && _isReadableText(child.name)) { + // Re-check the filtering that the initial search applied + if (_inSearchScope(child)) { + addedFiles.push(child); + addedFilePaths.push(child.fullPath); + } } + return true; } - return true; - } - return false; - }; + return false; + }; - entry.visit(visitor, function (err) { - if (err) { - deferred.reject(err); - return; - } + entry.visit(visitor, function (err) { + if (err) { + deferred.reject(err); + return; + } - //node Search : inform node about the file changes - filesChanged(addedFilePaths); + //node Search : inform node about the file changes + //filesChanged(addedFilePaths); + fullPaths = fullPaths.concat(addedFilePaths); - if (findOrReplaceInProgress) { - // find additional matches in all added files - Async.doInParallel(addedFiles, function (file) { - return _doSearchInOneFile(file) - .done(function (foundMatches) { - resultsChanged = resultsChanged || foundMatches; - }); - }).always(deferred.resolve); - } - }); + if (findOrReplaceInProgress) { + // find additional matches in all added files + Async.doInParallel(addedFiles, function (file) { + return _doSearchInOneFile(file) + .done(function (foundMatches) { + resultsChanged = resultsChanged || foundMatches; + }); + }).always(deferred.resolve); + } else { + deferred.resolve(); + } + }); - return deferred.promise(); + return deferred.promise(); + }).always(function () { + // this should be called once with a large array instead of numerous calls with single items + filesChanged(fullPaths); + }); } if (!entry) { @@ -804,23 +830,23 @@ define(function (require, exports, module) { var addPromise; if (entry.isDirectory) { - if (!added || !removed || (added.length === 0 && removed.length === 0)) { + if (added.length === 0 && removed.length === 0) { // If the added or removed sets are null, must redo the search for the entire subtree - we // don't know which child files/folders may have been added or removed. - _removeSearchResultsForEntry(entry); + _removeSearchResultsForEntries([ entry ]); var deferred = $.Deferred(); addPromise = deferred.promise(); entry.getContents(function (err, entries) { - Async.doInParallel(entries, _addSearchResultsForEntry).always(deferred.resolve); + _addSearchResultsForEntries(entries).always(deferred.resolve); }); } else { - removed.forEach(_removeSearchResultsForEntry); - addPromise = Async.doInParallel(added, _addSearchResultsForEntry); + _removeSearchResultsForEntries(removed); + addPromise = _addSearchResultsForEntries(added); } } else { // entry.isFile - _removeSearchResultsForEntry(entry); - addPromise = _addSearchResultsForEntry(entry); + _removeSearchResultsForEntries([ entry ]); + addPromise = _addSearchResultsForEntries([ entry ]); } addPromise.always(function () { @@ -830,6 +856,56 @@ define(function (require, exports, module) { } }); }; + + /** + * This stores file system events emitted by watchers that were not yet processed + */ + var _cachedFileSystemEvents = []; + + /** + * Debounced function to process emitted file system events + * for cases when there's a lot of fs events emitted in a very short period of time + */ + _processCachedFileSystemEvents = _.debounce(function () { + // we need to reduce _cachedFileSystemEvents not to contain duplicates! + _cachedFileSystemEvents = _cachedFileSystemEvents.reduce(function (result, obj) { + var fullPath = obj.entry ? obj.entry.fullPath : null; + // merge added & removed + if (result[fullPath] && obj.isDirectory) { + obj.added = obj.added.concat(result[fullPath].added); + obj.removed = obj.removed.concat(result[fullPath].removed); + } + // use the latest event as base + result[fullPath] = obj; + return result; + }, {}); + _.forEach(_cachedFileSystemEvents, function (obj) { + _fileSystemChangeHandler(obj.event, obj.entry, obj.added, obj.removed); + }); + _cachedFileSystemEvents = []; + }, FILE_SYSTEM_EVENT_DEBOUNCE_TIME); + + /** + * Wrapper function for _fileSystemChangeHandler which handles all incoming fs events + * putting them to cache and executing a debounced function + */ + _debouncedFileSystemChangeHandler = function (event, entry, added, removed) { + // normalize this here so we don't need to handle null later + var isDirectory = false; + if (entry && entry.isDirectory) { + isDirectory = true; + added = added || []; + removed = removed || []; + } + _cachedFileSystemEvents.push({ + event: event, + entry: entry, + isDirectory: isDirectory, + added: added, + removed: removed + }); + _processCachedFileSystemEvents(); + }; /** * On project change, inform node about the new list of files that needs to be crawled.