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

From: JD Smith <[email-masked]>
Date: Tue, 8 Apr 2025 09:46:13 -0400

> On Apr 8, 2025, at 4:00 AM, João Távora <[email-masked]> wrote:
> 
> JD Smith <[email-masked]> writes:
> 
> You pull them when Flymake decides it's a good time to pull, and that's it.

I see.  What I was thinking here is that since flymake doesn't understand that it is talking with an LSP server, it may ask for updates at inconvenient times.  Ideally you'd arrange the timing of diagnostic pulls to maximize the chances the document version matches. 

>> * 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.

I meant LSP version identifiers.  After my success solving the current problem using them, I believe ensuring the document version the LSP server worked against when working up a response to the diagnostic pull request matches the current version is vitally important.  Luckily this looks to be straightforward.

>> 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'll take it under consideration, though I suspect it would be 10x easier for someone more familiar with eglot's semantics and code conventions, which are more complex than typical packages.

>> I will open a separate bug for that.
> 
> Paste the number here when you do that.

Opened at bug#77620.

>> 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.

Thanks.  This tests fine on master.  Performance in my large python buffer is now refreshingly acceptable.  Two questions:

- Can we be certain that `eglot--versioned-identifier' is always an integer?
- Any reason not to implement this on the emacs-30 branch?  

> 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.  

That's fine, it likely won't matter since that code will now rarely be triggered, and possibly thingatpt performance can be improved in python-mode for those rare times in which it is.

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

I don't think the LSP server is to blame here.  Yes, it's sending large messages (too often) and getting behind, but it is dutifully providing the document version number.  The client is presumably responsible to act on that accordingly.  That said, I suspect eglot predates the availability of this particular version information.

Thanks for your work on eglot.  Used daily.