Skip to content

Limit search response size to avoid crash/perf issues#1753

Merged
dzhou121 merged 1 commit intolapce:masterfrom
MinusGix:search-limit-size
Nov 26, 2022
Merged

Limit search response size to avoid crash/perf issues#1753
dzhou121 merged 1 commit intolapce:masterfrom
MinusGix:search-limit-size

Conversation

@MinusGix
Copy link
Copy Markdown
Member

@MinusGix MinusGix commented Nov 25, 2022

  • Added an entry to CHANGELOG.md if this change could be valuable to users

There was a bug where searching in a workspace with files with large one-liner text would cause the editor to die (sometimes crash, sometimes visual artifacts due to creating a too large text layout and maybe messing with GPU memory or something). A common example would be minified javascript files, which are on a singular line and are quite massive.
The issue was that the search would respond with the entire line's text, which could be a lot (ex: one of my files was 500kb of text in a single line)! This wouldn't be too bad by itself, though it would probably stress search performance if you had a lot of them, but the main issue comes from trying to render it. It would try creating a text layout, which would then cause issues due to it being so large.

The fix is just to take a part of the line, 100 characters before and 100 characters after the end of the match, and restrict the text sent (and thus displayed) to that.

@MinusGix MinusGix added C-perf Category: performance A-search Area: searching text/lines/symbols labels Nov 25, 2022
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Nov 25, 2022

Codecov Report

Merging #1753 (eb7af1a) into master (4be5347) will decrease coverage by 0.00%.
The diff coverage is 0.00%.

@@            Coverage Diff            @@
##           master   #1753      +/-   ##
=========================================
- Coverage    6.54%   6.54%   -0.01%     
=========================================
  Files         127     127              
  Lines       54089   54100      +11     
=========================================
  Hits         3541    3541              
- Misses      50548   50559      +11     
Impacted Files Coverage Δ
lapce-proxy/src/dispatch.rs 0.00% <0.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@dzhou121 dzhou121 merged commit 3130a9c into lapce:master Nov 26, 2022
@panekj panekj added this to the Next release milestone Nov 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-search Area: searching text/lines/symbols C-perf Category: performance

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants