Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

100% CPU usage in file picker #6697

Closed
antoyo opened this issue Apr 10, 2023 · 7 comments · Fixed by #7028
Closed

100% CPU usage in file picker #6697

antoyo opened this issue Apr 10, 2023 · 7 comments · Fixed by #7028
Labels
C-bug Category: This is a bug

Comments

@antoyo
Copy link
Contributor

antoyo commented Apr 10, 2023

Summary

Hi.
I don't have a way for you to reproduce this issue (edit: now I do, see below), but when I type some specific string (in my case build_) in one of my projects, the editor freezes and uses 100% CPU. perf top shows that this happens in the function tree_sitter_rescript_external_scanner_scan.

Since I don't have a way to reproduce this issue, I can attempt to debug it, but I'd need some guidance.

Thanks.

Reproduction Steps

I tried this:

  1. Download this file in a directory and rename it to res.
  2. hx
  3. <Space>f to open the file picker
  4. Type res or whatever is needed to select the above file.

I expected this to happen:

Not freeze and not use 100% CPU.

Instead, this happened:

Helix freezes and uses 100% CPU.

Helix log

Nothing in the logs.

Platform

Linux

Terminal Emulator

alacritty 0.12.0 (5a728195)

Helix Version

helix 23.03

@antoyo antoyo added the C-bug Category: This is a bug label Apr 10, 2023
@antoyo
Copy link
Contributor Author

antoyo commented Apr 10, 2023

This could be a duplicate of #5871.

@pascalkuthe
Copy link
Member

Helix automatically starts yntax highlighting the selected fuel after a short delay. This can theoretically result in huge slowdowns if we attempt to syntax highlight huge files (or there are bugs in the TS grammar).

It looks like you ran into this with the rwscript grammar .res files. Do you have.res files in your repo? Are they very large?

A fix for this could be to set a hard limit for syntax highlighting (like 50MB). This was discussed before (to avoid the editor crashing/slwoing to a crawl in such cases)

@antoyo
Copy link
Contributor Author

antoyo commented May 12, 2023

I do not have a .res file, but I do have a file named res that contains 783 KB of logs.
And that was the missing piece to be able to reproduce the issue. I can upload that file somewhere if needed.

Maybe the rwscript grammar should not be loaded in that case?
It looks like this is a bug in helix: opening a file named exactly the extension of a grammar, e.g. rs, py will load the grammar.

@pascalkuthe
Copy link
Member

pascalkuthe commented May 12, 2023

Hmm I think its in purpose that we can match filenames too. For example we use Makefile as a pattern to match makefiles or Cargo.lock for toml. The fix would probably be to convert the various pattern yo use a dot (so the pattern would be .rs instead of rs).

800KB is not that huge of a file tough. This is a bug in the grammar. The scannwrs are handwritten c code so they probably disnt probably fuzz/test their scanner. You could try to report it to the TS grammar repo. We can't do anything about that inside helix tough.

Tree sitter actually provides a way to set a flat timeout for parsing. We could set that to something large (like 1s) and if it's reached disable syntac highlighting for that file. That should be fairly easy to implement and would improve a lot of the "helix freezes because of TS bugs".

I am not 100% if it helps in this case because if the scanner gets stuck in an infinite loop inside handwritten c code then the TS timeout can't help either. But I guess those kinds of bugs really should be fixed in the grammars.

@pascalkuthe
Copy link
Member

pascalkuthe commented May 12, 2023

Also please do upload the file somewhere having reproducible examples is very useful

@antoyo
Copy link
Contributor Author

antoyo commented May 12, 2023

Here's the file:
res.txt

It will need to be renamed to res.

Btw, if renamed to rs (to use the Rust parser), that will freeze Helix for 2-3 seconds. Not as bad, but maybe there's something to do here.

Can TS be called asynchronously for the file picker?

@pascalkuthe
Copy link
Member

pascalkuthe commented May 12, 2023

usually async highlighting is hard (if typing) but for the picker it did turn out quite easy. To ensure we don't saturate a bunch of background tasks I still added the timeout I described above. It seems that rwscript scanner is just very slow and not actually freezing (as the TS timeout still works).

It might still be nice to change the various file extensions to include a dot in the language config and open an issue to the upstream REPO but #7028 completely fixes the freezing for me so that should address this issue.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
C-bug Category: This is a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants