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

Fix #218: Defer eglot-ensure until window-configuration-change #989

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

kimo-k
Copy link

@kimo-k kimo-k commented Jul 4, 2022

Some emacs users can open many buffers at a time.
Packages like desktop.el and persp-mode encourage this.
A user may also invoke eglot-ensure when each buffer starts.

The effect of this - spawning hundreds of language-server processes -
can crash the server and lag the system.

Now, eglot-ensure waits until window-configuration-change time
before starting eglot--managed-mode. Window-configuration-change
time is when the user interacts with the local buffer's window UI.

Used João Távora's idea to apply a pattern from flymake:
https://github.com/emacs-straight/flymake/blob/6bc8782d9c05d4c9bbba524542d457e6a3b1734b/flymake.el#L1013

Copyright-paperwork-exempt: Yes

Some emacs users can open many buffers at a time.
Packages like `desktop.el` and `persp-mode` encourage this.
A user may also invoke eglot-ensure when each buffer starts.

The effect of this - spawning hundreds of language-server processes -
can crash the server and lag the system.

Now, `eglot-ensure` waits until window-configuration-change time
before starting `eglot--managed-mode`. Window-configuration-change
time is when the user interacts with the local buffer's window UI.

Used João Távora's idea to apply a pattern from flymake:
https://github.com/emacs-straight/flymake/blob/6bc8782d9c05d4c9bbba524542d457e6a3b1734b/flymake.el#L1013

Copyright-paperwork-exempt: Yes
@joaotavora
Copy link
Owner

This is nice, thanks. Some notes:

  1. the description

    Though pointing in a familiar direction, the existing description it doesn't really clarify how the problem this purports to be solving can arise in the first place. It just talks about the preconditions (i.e. finding many files at once) and the post conditions (hang). But the in-between is fuzzy..

    It's important to clarify this because this is a non-trivial change that's not obvious to understand for future maintainers. Also, it's not immediately clear that the problem is completely solved here.

    In my view -- but this is only conjecture -- the problem is that eglot-connect is potentially asynchronous, i.e. non-blocking. If it were always blocking, then one buffer's attempt would block until if eventually got the server, run eglot--maybe-activate-editing-mode in all buffers, and then all the other attempts at eglot-connect for buffers in the same project as the first attempt would not happen. So, logically, it would mean that the problem is caused by non-t eglot-sync-connect.

    But, again, this is only conjecture. I need you to confirm that this is also your view of the potential problem. If it's not, clearly explain how the preconditions develop into the post conditions. Ideally, describe an experiment that I or others can run.

  2. that it is not really copyright exempt.

    It's too many code/comments and logic, I think. I'm not sure, and we can ping @skangas if he's around. Either you get an FSF copyright assignment, or I can re-do your work and credit you for the suggestion.

  3. this is missing references to related issues

    If this problem is the root cause of one/several "Eglot hangs" issues around the bug tracker, it'd be great to know about them. I think it may be (and, what's more, the eglot-sync-connect thing would explain why it only manifests itself for "large" projects).

  4. problems in this solution.

    even with the "window configuration defer" technique, what if the user aggressively switches to many buffers in the same project and all end up launching eglot-connect requests? Confusion still? I would guess so.

  5. alternative solutions

    in light of 1, if my conjecture is confirmed, I wonder if simply setting eglot-sync-connect to t in eglot-ensure doesn't effectively also solve this problem.

    or maybe, much more complex, but there could be a third state in eglot--servers-by-project where one marks that the server is being initialized by one of the buffers in the project. Or maybe aggressively store un-initialized servers in there, and clean up afterwards if initialization has failed.

  6. I kind of regret having added eglot-ensure, but users seem to love it 🤷. Is remembering/typing M-x eglot when starting to work on a project really that much work?

@joaotavora
Copy link
Owner

joaotavora commented Jul 4, 2022

I just realized that indeed I just suggested this 3 years ago #218 (comment) :-) Well, I didn't think of all the consequences (as one usually doesn't :-) )

@manuel-uberti
Copy link
Contributor

6. I kind of regret having added `eglot-ensure`, but users seem to love it shrug.  Is remembering/typing `M-x eglot` when starting to work on a project really that much work?

FWIW, after some time with major mode hooks to run eglot-ensure, I dropped them and opted for M-x eglot when I need it. You're right: it's that easy. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants