Merge pull request #5303 from adobe/pflynn/find-highlight-perf

gruehle · gruehle · commit 8e79f54e8a2b · 2013-09-26T09:49:07.000-07:00
Performance improvements for initial keystrokes in Find
diff --git a/src/search/FindReplace.js b/src/search/FindReplace.js
@@ -52,8 +52,14 @@ define(function (require, exports, module) {
     var searchReplacePanelTemplate   = require("text!htmlContent/search-replace-panel.html"),
         searchReplaceResultsTemplate = require("text!htmlContent/search-replace-results.html");
 
-    /** @cost Constant used to define the maximum results to show */
-    var FIND_REPLACE_MAX    = 300;
+    /** @const Maximum file size to search within (in chars) */
+    var FIND_MAX_FILE_SIZE  = 500000;
+
+    /** @const If the number of matches exceeds this limit, inline text highlighting and scroll-track tickmarks are disabled */
+    var FIND_HIGHLIGHT_MAX  = 2000;
+
+    /** @const Maximum number of matches to collect for Replace All; any additional matches are not listed in the panel & are not replaced */
+    var REPLACE_ALL_MAX     = 300;
 
     /** @type {!Panel} Panel that shows results of replaceAll action */
     var replaceAllPanel = null;
@@ -152,6 +158,7 @@ define(function (require, exports, module) {
             });
         });
         state.marked.length = 0;
+        
         ScrollTrackMarkers.clear();
     }
 
@@ -206,13 +213,6 @@ define(function (require, exports, module) {
         
         ScrollTrackMarkers.setVisible(editor, enabled);
     }
-    
-    function addHighlight(editor, state, cursor) {
-        var cm = editor._codeMirror;
-        state.marked.push(cm.markText(cursor.from(), cursor.to(), { className: "CodeMirror-searching" }));
-        
-        ScrollTrackMarkers.addTickmark(editor, cursor.from());
-    }
 
     /**
      * If no search pending, opens the search dialog. If search is already open, moves to
@@ -264,33 +264,44 @@ define(function (require, exports, module) {
                 //Flag that controls the navigation controls.
                 var enableNavigator = false;
                 
-                // Highlight all matches
+                // Find all matches
                 // (Except on huge documents, where this is too expensive)
-                if (cm.getValue().length < 500000) {
-                    toggleHighlighting(editor, true);
-                    
+                var resultSet = [];
+                if (cm.getValue().length <= FIND_MAX_FILE_SIZE) {
                     // FUTURE: if last query was prefix of this one, could optimize by filtering existing result set
-                    var resultCount = 0;
                     var cursor = getSearchCursor(cm, state.query);
                     while (cursor.findNext()) {
-                        addHighlight(editor, state, cursor);
-                        resultCount++;
-
-                        //Remove this section when https://github.com/marijnh/CodeMirror/issues/1155 will be fixed
+                        resultSet.push(cursor.pos);  // pos is unique obj per search result
+                        
+                        // TODO: remove this section when https://github.com/marijnh/CodeMirror/issues/1155 is fixed
                         if (cursor.pos.match && cursor.pos.match[0] === "") {
                             if (cursor.to().line + 1 === cm.lineCount()) {
                                 break;
                             }
                             cursor = getSearchCursor(cm, state.query, {line: cursor.to().line + 1, ch: 0});
                         }
                     }
-                                        
-                    if (resultCount === 0) {
+                    
+                    // Highlight all matches if there aren't too many
+                    if (resultSet.length <= FIND_HIGHLIGHT_MAX) {
+                        toggleHighlighting(editor, true);
+                        
+                        resultSet.forEach(function (result) {
+                            state.marked.push(cm.markText(result.from, result.to, { className: "CodeMirror-searching" }));
+                        });
+                        var scrollTrackPositions = resultSet.map(function (result) {
+                            return result.from;
+                        });
+                        
+                        ScrollTrackMarkers.addTickmarks(editor, scrollTrackPositions);
+                    }
+                    
+                    if (resultSet.length === 0) {
                         $("#find-counter").text(Strings.FIND_NO_RESULTS);
-                    } else if (resultCount === 1) {
+                    } else if (resultSet.length === 1) {
                         $("#find-counter").text(Strings.FIND_RESULT_COUNT_SINGLE);
                     } else {
-                        $("#find-counter").text(StringUtils.format(Strings.FIND_RESULT_COUNT, resultCount));
+                        $("#find-counter").text(StringUtils.format(Strings.FIND_RESULT_COUNT, resultSet.length));
                         enableNavigator = true;
                     }
 
@@ -427,7 +438,7 @@ define(function (require, exports, module) {
                 post:      multiLine ? "\u2026" : line.slice(to.ch)
             });
 
-            if (results.length >= FIND_REPLACE_MAX) {
+            if (results.length >= REPLACE_ALL_MAX) {
                 break;
             }
         }
@@ -438,7 +449,7 @@ define(function (require, exports, module) {
                 Strings.FIND_REPLACE_TITLE_PART3,
                 resultsLength,
                 resultsLength > 1 ? Strings.FIND_IN_FILES_MATCHES : Strings.FIND_IN_FILES_MATCH,
-                resultsLength >= FIND_REPLACE_MAX ? Strings.FIND_IN_FILES_MORE_THAN : ""
+                resultsLength >= REPLACE_ALL_MAX ? Strings.FIND_IN_FILES_MORE_THAN : ""
             );
 
         // Insert the search summary
diff --git a/src/search/ScrollTrackMarkers.js b/src/search/ScrollTrackMarkers.js
@@ -78,14 +78,16 @@ define(function (require, exports, module) {
         trackHt -= trackOffset * 2;
     }
 
-    /** Add one tickmark to the DOM */
-    function _renderMark(pos) {
-        var top = Math.round(pos.line / editor.lineCount() * trackHt) + trackOffset;
-        top--;  // subtract ~1/2 the ht of a tickmark to center it on ideal pos
-        
-        var $mark = $("<div class='tickmark' style='top:" + top + "px'></div>");
-        
-        $(".tickmark-track", editor.getRootElement()).append($mark);
+    /** Add all the given tickmarks to the DOM in a batch */
+    function _renderMarks(posArray) {
+        var html = "";
+        posArray.forEach(function (pos) {
+            var top = Math.round(pos.line / editor.lineCount() * trackHt) + trackOffset;
+            top--;  // subtract ~1/2 the ht of a tickmark to center it on ideal pos
+            
+            html += "<div class='tickmark' style='top:" + top + "px'></div>";
+        });
+        $(".tickmark-track", editor.getRootElement()).append($(html));
     }
     
     
@@ -128,9 +130,7 @@ define(function (require, exports, module) {
                 if (marks.length) {
                     _calcScaling();
                     $(".tickmark-track", editor.getRootElement()).empty();
-                    marks.forEach(function (pos) {
-                        _renderMark(pos);
-                    });
+                    _renderMarks(marks);
                 }
             }));
             
@@ -143,16 +143,20 @@ define(function (require, exports, module) {
         }
     }
     
-    /** Add a tickmark to the editor's tickmark track, if it's visible */
-    function addTickmark(curEditor, pos) {
+    /**
+     * Add tickmarks to the editor's tickmark track, if it's visible
+     * @param curEditor {!Editor}
+     * @param posArray {!Array.<{line:Number, ch:Number}>}
+     */
+    function addTickmarks(curEditor, posArray) {
         console.assert(editor === curEditor);
         
-        marks.push(pos);
-        _renderMark(pos);
+        marks = marks.concat(posArray);
+        _renderMarks(posArray);
     }
     
     
     exports.clear = clear;
     exports.setVisible = setVisible;
-    exports.addTickmark = addTickmark;
+    exports.addTickmarks = addTickmarks;
 });
