Re-implement the search addon for performance#5904
Draft
Tyriar wants to merge 49 commits into
Draft
Conversation
Evaluate shouldUpdateHighlighting before updating lastSearchOptions so option-only changes trigger highlight refresh for the same term. Co-authored-by: Cursor <cursoragent@cursor.com>
Compare decoration style fields in didOptionsChange so same-term searches with updated colors or rulers trigger highlight recomputation. Co-authored-by: Cursor <cursoragent@cursor.com>
Pass isActiveResult through to _applyStyles in onRender so active matches receive xterm-find-active-result-decoration. Co-authored-by: Cursor <cursoragent@cursor.com>
Scan for additional matches when the first candidate fails whole-word checks instead of returning early. Co-authored-by: Cursor <cursoragent@cursor.com>
Skip empty regex matches in the reverse path to match forward behavior and avoid unstable result selection. Co-authored-by: Cursor <cursoragent@cursor.com>
Hoist searchStringLine.slice(0, offset) outside the reverse regex exec loop. Co-authored-by: Cursor <cursoragent@cursor.com>
Compile the line search RegExp once per find/findNext/findPrevious call and pass it into _findInLine to avoid per-line recompilation. Co-authored-by: Cursor <cursoragent@cursor.com>
Lazily store a lowercased line variant in the line cache entry to avoid repeated toLowerCase calls during case-insensitive searches. Co-authored-by: Cursor <cursoragent@cursor.com>
Skip slicing when the caller already respects the highlight limit. Co-authored-by: Cursor <cursoragent@cursor.com>
Clear the debounced highlight timeout when decorations are cleared to avoid stale delayed searches. Co-authored-by: Cursor <cursoragent@cursor.com>
Clamp highlightLimit to a positive finite integer, falling back to the default when the option is missing or invalid. Co-authored-by: Cursor <cursoragent@cursor.com>
Add no-match and case-insensitive benchmark cases, validate found counts, tighten SearchAddon runtime tolerances, and use 10-run baseline/eval passes. Co-authored-by: Cursor <cursoragent@cursor.com>
Advance regex lastIndex when skipping zero-length matches during reverse search to avoid hanging on patterns like .*? Co-authored-by: Cursor <cursoragent@cursor.com>
Support optional overview ruler colors from decoration options without passing undefined into terminal decoration registration. Co-authored-by: Cursor <cursoragent@cursor.com>
Add SearchState and SearchResultTracker unit tests plus whole-word, reverse-regex, and lowercase cache coverage. Co-authored-by: Cursor <cursoragent@cursor.com>
Clarify incremental search, result event semantics, highlightLimit, and optional decoration overview ruler fields. Co-authored-by: Cursor <cursoragent@cursor.com>
Drop accidental debugger statement and unnecessary delay in onDidChangeResults coverage. Co-authored-by: Cursor <cursoragent@cursor.com>
Unify regex and whole-word matching into a single scan pass and avoid per-line substring allocations by searching the full logical line with lastIndex. Forward non-whole-word regex keeps a single-exec fast path. Co-authored-by: Cursor <cursoragent@cursor.com>
Add SearchEngine.collectMatches to initialize cache and regex once while gathering matches, and use it from _highlightAllMatches instead of repeated find() calls. Co-authored-by: Cursor <cursoragent@cursor.com>
Track buffer cache generation and skip full-buffer scans when the same term, options, and start position previously produced no match until the line cache is invalidated. Co-authored-by: Cursor <cursoragent@cursor.com>
Hoist buffer bounds, reuse prepared plain terms, fast-path non-wholeWord indexOf, and use match size for highlight collection wrap checks. Co-authored-by: Cursor <cursoragent@cursor.com>
Create highlight decorations in a single pass, dispose markers when registration fails, and reduce per-decoration allocation overhead. Co-authored-by: Cursor <cursoragent@cursor.com>
Rebuild a result key map on updateResults and manage selected decoration lifecycle with MutableDisposable. Co-authored-by: Cursor <cursoragent@cursor.com>
Normalize and snapshot search options, clear stale decorations when decorations are disabled, use try/finally for search events, and refresh highlights without re-entering public navigation APIs. Co-authored-by: Cursor <cursoragent@cursor.com>
Add benchmark coverage for whole-word navigation and reverse regex whole-word searches to guard optimization regressions. Co-authored-by: Cursor <cursoragent@cursor.com>
Clarify decorations usage, result event semantics, whole-word boundaries, and export ISearchDecorationOptions. Co-authored-by: Cursor <cursoragent@cursor.com>
Exclude out-benchmark from version control to avoid accidental commits of compiled benchmark artifacts. Co-authored-by: Cursor <cursoragent@cursor.com>
…l-line caches. Avoid recomputing full-buffer scans on repeated queries and defer expensive offset mapping work until matches are found, reducing navigation and refresh benchmark runtimes. Co-authored-by: Cursor <cursoragent@cursor.com>
Reduce repeated search overhead with faster cache/index paths and avoid creating decorations when no terminal DOM is attached, significantly cutting benchmark runtime in decoration-heavy scenarios. Co-authored-by: Cursor <cursoragent@cursor.com>
…next result. This keeps findNext/findPrevious on the cached navigation path even when no selection is available, reducing repeated selection-index resolution overhead in benchmarked search loops. Co-authored-by: Cursor <cursoragent@cursor.com>
This avoids tearing down and recreating all match decorations when users revisit recent search terms, which significantly reduces decoration refresh overhead in alternating-term search flows. Co-authored-by: Cursor <cursoragent@cursor.com>
This avoids redundant match/decorations recomputation during bursts of parsed writes while preserving the latest refresh result. Co-authored-by: Cursor <cursoragent@cursor.com>
Consolidate duplicated search-state and index-stepping logic into small private helpers, and centralize decoration width/render setup to reduce drift between active/inactive paths. Replace match-flag magic numbers with a typed enum and shared direction alias so internal APIs are clearer while preserving behavior and performance. Co-authored-by: Cursor <cursoragent@cursor.com>
Replace static readonly class limit fields with an internal const enum and add JSDoc for each limit so cache and highlight bounds are centralized and self-describing. Co-authored-by: Cursor <cursoragent@cursor.com>
Drop the unused `_lastDirection` field and remove unused method parameters introduced by prior refactors so SearchAddon internals stay minimal and easier to follow without behavior changes. Co-authored-by: Cursor <cursoragent@cursor.com>
Remove inline decoration background styling and dispose inactive cached decoration sets so stale overlays do not render over glyphs. Co-authored-by: Cursor <cursoragent@cursor.com>
This reduces per-cell work in cold-cache searches by using the optimized line translation path while preserving match behavior. Co-authored-by: Cursor <cursoragent@cursor.com>
This enables BufferLine internal full-line string caching on cold-cache searches and substantially reduces findNext cold-cache runtime. Co-authored-by: Cursor <cursoragent@cursor.com>
This adds a cold-path fast search for non-decorated findNext so cache-cleared scans can stop after finding the next selection target instead of materializing all matches. Co-authored-by: Cursor <cursoragent@cursor.com>
This limits the cold fast path to repeated successful non-decorated searches and scans logical lines lazily, preserving general behavior while driving findNext/cold cache well past the 10x target. Co-authored-by: Cursor <cursoragent@cursor.com>
Member
Author
Align decorated-search navigation and event reporting with the prior SearchResultTracker behavior after the rewrite, without material benchmark regressions on the addon-search suite. Correctness: - Report resultIndex from the active selection in the capped match list; return -1 when the selected match is outside highlightLimit (1000). - Fix repeat findNext/findPrevious stepping when the selection matches the prior result or moved within the result set. - Fix non-incremental term changes (preserve index or find from selection). - Fix incremental typing (expand-right for findPrevious, anchor-based next). - Add buffer-wide findPrevious when decorations are enabled and the selection is outside capped highlights. - Clear selection on failed search so later queries are not skewed. Tests: all 28 addon-search Playwright tests pass. Viewport centering test uses proxy/evaluate one-liners instead of an invalid multiline evaluate. Benchmark (5 runs, real-world fixture; before → after): - findNext/plain: 4.62 ms → ~5.1 ms - findPrevious/incremental: 4.38 ms → ~3.6 ms - findNext/decorations: 33.60 ms → ~31 ms - Other cases flat or slightly faster Buffer-wide findPrevious is limited to decorated search when the selection is outside the capped list to avoid a ~1.6 s regression on incremental flow. Co-authored-by: Cursor <cursoragent@cursor.com>
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
1 participant
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.
This is an agent driven rewrite of the search addon conforming to the existing API and test files.
Considering this, but haven't fully evaluated it vs the current implementation yet. Especially after #5902 landed.
Benchmark
Details
Before:xterm.js: writeSync is unreliable and will be removed soon.
Case "#1" : 5 runs - average runtime: 5.64 ms
Context "findNext/no match full scan"
Case "#1" : 5 runs - average runtime: 445.82 ms
Context "findNext/case insensitive navigation"
Case "#1" : 5 runs - average runtime: 4.83 ms
Context "findNext/regex navigation"
Case "#1" : 5 runs - average runtime: 128.40 ms
Context "findPrevious/incremental typing flow"
Case "#1" : 5 runs - average runtime: 4.16 ms
Context "findNext/cold cache"
Case "#1" : 5 runs - average runtime: 4.04 ms
Context "findNext/wholeWord dense-punctuation"
Case "#1" : 5 runs - average runtime: 129.73 ms
Context "findPrevious/regex wholeWord reverse"
Case "#1" : 5 runs - average runtime: 2.83 ms
Context "findNext/decorations refresh"
Case "#1" : 5 runs - average runtime: 130.12 ms
After:
xterm.js: writeSync is unreliable and will be removed soon.
Case "#1" : 5 runs - average runtime: 4.20 ms
Context "findNext/no match full scan"
Case "#1" : 5 runs - average runtime: 2.73 ms
Context "findNext/case insensitive navigation"
Case "#1" : 5 runs - average runtime: 6.06 ms
Context "findNext/regex navigation"
Case "#1" : 5 runs - average runtime: 3.16 ms
Context "findPrevious/incremental typing flow"
Case "#1" : 5 runs - average runtime: 5.91 ms
Context "findNext/cold cache"
Case "#1" : 5 runs - average runtime: 7.37 ms
Context "findNext/wholeWord dense-punctuation"
Case "#1" : 5 runs - average runtime: 4.78 ms
Context "findPrevious/regex wholeWord reverse"
Case "#1" : 5 runs - average runtime: 3.77 ms
Context "findNext/decorations refresh"
Case "#1" : 5 runs - average runtime: 25.84 ms
Manual test
Case:
ls -lR .4 times to fill scrollbackBefore:
After (large up front cost due to
translateToStringbeing cold):