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

From: João Távora <[email-masked]>
Date: Mon, 07 Apr 2025 17:00:13 +0100

Eli Zaretskii <[email-masked]> writes:

>> It doesn't take too many "botched eglot ranges" interacting with slow
>> `thingatpt' misbehavior to add up to a 10s delay.

ACK.
 
>> I'm not certain of the best solution.  A few ideas, from hardest to easiest:
>> 
>> Teach eglot textDocument/diagnostic
>> ++++++++++++++++++++++++++++
>> 
>>  textDocument/publishDiagnostics messages arrive way too frequently IMO, after every buffer change of any
>> kind.  They are pushed to eglot from the LSP server, and if they contain hundreds of errors, this becomes
>> very inefficient (re-painting with flymake the same hundreds of regions over and over after each keystroke).
>> 
>> The best solution here would probably be to adopt "pull" diagnostics using textDocument/diagnostic, perhaps
>> in an idle-timer whose duration the user can configure.  I don't believe EGLOT can do diagnostic pulls at the
>> moment.

You're right it can't.  A patch that implements without much code
repetition and keeps support for the "push" diagnostics model is
welcome.  Would you like to work on it, JD?

As far as I understand, this model is much more complicated and allows
you to pull diagnostics for individual LSP documents or the whole
project.  One of the difficulties I envison is to do it in a way that
maintains support for project-wide diagnostics.

But its certainly not impossible and would be a wonderful addition,
fixing many problems such as
https://github.com/joaotavora/eglot/issues/1296.

>> 
>> Don't use thingatpt in `flymake-diag-region'
>> +++++++++++++++++++++++++++++++++
>> 
>> `flymake-diag-region' should perhaps not use thingapt, which is
>> subject to the performance vagaries of the major-mode and underlying
>> file.  I am uncertain why it relies on that.  Perhaps the performance
>> of those will be improved with treesitter variants.

When a Flymake backend passes on to Flymake a 0-dimensional point in a
file you still want Flymake to create a diagnostic emcompassing a
1-dimensional span of buffer positions.

thingatpt.el is, AFAICT, the standard Emacs's way to move from the 0
dimension to the 1 dimension space.  It needs, quite understandibly,
help from the major mode to do that job.

If that help comes at a dog slow pace, I think that's a problem in
itself.

It takes around 10~20 microsseconds on my machine in Emacs Lisp mode as
measured by:

(/ (car (benchmark-run 10000 (thing-at-point 'sexp))) 10000)

Even less in c++-ts-mode, around 4 microsseconds.

So if python-mode is taking around a hundred thousand times (!) more to
compute that piece of information for some buffer positions, I'd say
it's definitely something to look at...

>From a quick check it seems indeed that the time it takes to compute it
is proportional to the value of point the buffer.  Suggest creating a
bug for python-mode, or calling in whoever wrote that part.
 
>> Eglot could detect off-by-one diagnostics
>> +++++++++++++++++++++++++++++++
>> 
>> Hard to know the best heuristic, but lots of null effective ranges is
>> a good one.
>> 
>> Eglot can simply ignore null range diagnostics
>> +++++++++++++++++++++++++++++++++++
>> 
>> Eglot doesn't need to use `flymake-diag-region' to try to calculate an update range if it encounters a null
>> diagnostic range.  It could simply drop those, as they are probably wrong anyway, and will shortly be updated.

Maybe, IF we can confidently say that the server is in the wrong. 

But I added it there for a reason, and quite a long time ago. See the
commit log for

commit 7826b265a0ecd9357719b2fb9491c9bcb517d4cc
Author: João Távora <[email-masked]>
Date:   Thu Jun 21 23:32:14 2018 +0100

I'm moderately sure someone somewhere expects the botches ranges to be
auto-corrected by Eglot (though admittedly not at the expense of large
delays).

João