Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Don't block the UI for prompt completions #11787

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rhogenson
Copy link
Contributor

@rhogenson rhogenson commented Sep 27, 2024

The prompt handler for : commands currently blocks the UI while generating completions, which can make the editor feel laggy when using :o or :cd on networked filesystems. I was reproducing this issue using github.com/nirs/slowfs to inject a 1 second delay to readdir() calls (although in reality I experienced this issue while working in google's monorepo).

To see the difference, I recorded before and after videos of navigating a slowfs directory containing some nested directories.

Before the change: https://asciinema.org/a/yzdSZUjjnrBPAU1VcFcVAkLyu

After the change: https://asciinema.org/a/fgayEu1ZczVwqnMrAsmsvuPrJ

Fixes #11604

Copy link
Member

@the-mikedavis the-mikedavis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have looked into this refactor in the past and couldn't find a way I really liked to solve it. We definitely want to move the I/O done for path completion off the main thread but the refactor to do so is not straightforward.

Refactoring Completer to drop the Editor arg and use jobs/callbacks has drawbacks. It's a very large change that affects nearly any place that we use completers (this part we may not be able to avoid). It's also pure overhead for all completers except path completion - fuzzy matching is so fast that doing it in the render loop is not worrisome. Plus it makes completers that need to interact with the Editor complicated/unnatural (for example buffer and language completers using a channel).

I chatted with @pascalkuthe about this in the past and he thought it might be a good idea to special-case the path completions or at least have a special case for any "deferred" completion values. As a straightforward way to do this we could have the Completer type return an enum that either is the full completion Vec<Completion> or a deferred value that would be debounced and calculated off the main thread in a hook.

@rhogenson
Copy link
Contributor Author

Thank you! I was having trouble finding a good way to handle both synchronous and asynchronous completions, but that works really well! I think this change turned out a lot nicer, please take a look.

This change allows prompt completions to be calculated in the
background, to avoid blocking the UI on slow file IO such as over a
networked FS.
@kirawi kirawi added A-helix-term Area: Helix term improvements S-waiting-on-review Status: Awaiting review from a maintainer. labels Oct 14, 2024
@archseer archseer added this to the next milestone Oct 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-helix-term Area: Helix term improvements S-waiting-on-review Status: Awaiting review from a maintainer.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Slow autocomplete when opening files over SSHFS
4 participants