Repository navigation
Fix quadratic search on long wrapped lines and whole-word misses - #6252
Open
ahmadsh2007 wants to merge 1 commit into
Open
ahmadsh2007 wants to merge 1 commit into
ahmadsh2007 wants to merge 1 commit into
Conversation
Contributor
|
I believe the quadratic search problem is avoided by PR #6160. For whole-word searches using a regex, we could simplify the logic by modifying the regex: For non-regex searches, it probably makes sense to just escape the search string, regardless of whether doing a while-word search, as this would further simplify the logic: However, there is a problem in that |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Title: Fix quadratic search on long wrapped lines and whole-word misses in search addon
Fixes #4902
Problem
1. Search is quadratic in the length of a wrapped line. The forward loops in
find()andfindNextWithSelection()call_findInLineon every buffer row. For a continuation row (isWrapped),_findInLinerecurses back up to the row where the line starts. Each time, it recomputes the string offset cell by cell (_bufferColsToStringOffset), lowercases the whole joined line again, and searches the rest of it again. A line wrapping over k rows is therefore processed about k times, at a cost of O(k) each. Searching the row where the line starts already covers every continuation row, so this work is redundant.2. Whole-word search only checks the first occurrence on a line. If the first occurrence of the term is part of a larger word,
_findInLinereturnsundefinedand never checks later occurrences. For example, whole-word search forfooinfoobar foofinds nothing. On wrapped lines this was hidden, because the redundant rescans from problem 1 happened to find the later match.Fix
_findInLinekeeps scanning past occurrences that aren't whole words, for both plain and regex search and in both directions. This is needed so that the first change doesn't regress whole-word matching on wrapped lines, and it also fixes the bug on normal lines.Results
Searching for a term that isn't present. Headless
Terminal, 80 cols, median of 5 runs:Before the fix, each doubling of line length makes search about 4× slower, which is the quadratic behavior described above. After the fix the time stays roughly flat.
"Highlight-all loop" is the loop from
SearchAddon._highlightAllMatches, run directly againstSearchEngine, since the addon itself needs a DOM.Tests
masterand pass with this change.Notes / possible follow-ups
^could previously match at the start of a wrapped continuation row, because each such row was searched as its own slice. It now matches only at the start of the logical line, which is what reverse search already did._bufferColsToStringOffsetwalking cells. That is unchanged and could be addressed separately.