Subject: bug#77588: Catastrophic slowdown in eglot via `flymake-diag-region'

From: João Távora <[email-masked]>
Date: Tue, 08 Apr 2025 09:00:55 +0100

JD Smith <[email-masked]> writes:

> know much about how this has been implemented in other clients, and there are many questions: 
>
> * When is the appropriate time to pull?
> * Should pull be done in idle time?  Driven by flymake?

You are overthinking things.  Pull diagnostics are actually much closer
to Flymake's original model than push diagnostics.  You pull them when
Flymake decides it's a good time to pull, and that's it.

  See https://www.gnu.org/software/emacs/manual/html_node/flymake/Backend-functions.html

> * When should you ask for the full project's diagnostics vs. just the
> current buffer?  Always?

You could start with just the current buffer.

> * Should diagnostics be versioned (likely)?

I don't know what that means, but the answer is likely no.  Flymake
knows how to manage its objects.

> As well, my familiarity with eglot/jsonrpc's internal structure and
> comm model is rudimentary at best.  I'd be happy to help with logic
> and provide deep testing, but I'm afraid it's not something I could
> tackle alone.

You don't need to know any jsonrpc internals to implement this.  As to
Eglot's internals, you need to know no more than what you've
demonstrated you already know in this issue.

> I will open a separate bug for that.

Paste the number here when you do that.

> The main issue is that eglot is trying to "correct" a region using
> data which is already outdated, and therefore not likely to result in
> anything useful, however hard flymake tries.

Agree.

> It seems eglot keeps track of the "document version" in `eglot--versioned-identifier'.  I believe that can be compared to
> the one in the `textDocument/diagnostic' message to avoid this mess
> altogether.

I've implemented this.  Please test and let's close.

The second thing you propose is too risky and redundant anyway.  Eglot
is generally not in the business of trying very hard to fix server
flaws.  So I will let the "botched ranged" code be.  It was introduced
early on when this policy was less clear and I wanted a young Eglot to
work with flawed servers.  I'll stay for backward compatibility reasons,
I've been doing this long enough to know someonw will grumble if I
delete it.

I suggest you open bugs against the LSP server (and the ridiculously
slow thingatpt.el for Python, which you already said you did)

João