On Thu, Apr 18, 2024 at 11:06 PM João Távora <joaotavora@gmail.com> wrote:
> Anyway is this the hotspot I should be trying to optimize?
>
> > 9 4% - find-buffer-visiting
Alright, i've reproduced this
33 5% - eglot-handle-notification
33 5% - apply
33 5% - #<compiled-function B79>
29 4% - find-buffer-visiting
29 4% - file-truename
29 4% - file-truename
25 4% - file-truename
25 4% - file-truename
25 4% - file-truename
25 4% - file-truename
25 4% + file-truename
But I have to say, I wouldn't call this a severe performance penalty.
I followed your instructions very closely and invoked Emacs like this:
emacs -Q foo/bar/baz/foo/bar/baz/foo/bar/baz/foo/bar/baz/gin/fs.go -f go-ts-mode
The directory is ~/tmp/theo/foo/bar... so it's a pretty long path with
many directories all in all.
But I didn't have to wait 10 seconds for the LSP to settle down! It was
pretty snappy on my 2018 Lenovo T480 running Archlinux. And if I
profile anything other than the initial M-x eglot (which normally happens
only once in a work session), I don't find any file-truename in the profile.
So my perception is that it must have spent around 4% of 1 second in
file-truename.
Anyway the reason this shows in this profile is because this project
with this particular server sends a lot of publishDiagnostics upfront.
That's OK. Gopls is a very good server. I think I see a fix. But can
you qualitatively describe the Eglot experience. Do you notice any
input lag or something like that? With this project? I didn't feel _any_ lag.
Super snappy. Maybe the JSON serde kicking in?
Anyway, the idea I suggested earlier is in the patch after my sig.
Let's think: LSP's publishDiagnostics coming from the server deals
in URIs, right? And we inform the LSP server about URIs, too, right?
So the URI is always LSP's idea of the resource identifier (and it likes
to have truename URI).
My last "better fix" commit records this URI in the buffer as a buffer
local variable eglot--cached-tdi and it has to do that for every didOpen.
So, to find an open buffer pertaining to a certain LSP's publishDiagnostics
it suffices in theory to go through all the buffers that have a non-nil cached
URI and compare that.
No need to convert from URI to file names, not for this job at least!
I tried this and it worked fine.
When I do that, the profile is completely free of those 4% that
bothered you.
I'm still testing this for edge cases and will sleep on it, but it seems
promisingly simple at least. I can't run unit tests right now, because
a recent adventurous commit by Stefan Monnier broke them all :-)
but I'm confident that will be fixed soon...
I hope you can try this patch.
João
diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el
index 90a607075d3..38a16b15e4c 100644
--- a/lisp/progmodes/eglot.el
+++ b/lisp/progmodes/eglot.el
@@ -2381,6 +2381,9 @@ eglot-handle-notification
(lambda ()
(remhash token (eglot--progress-reporters server))))))))))
+(defvar-local eglot--cached-tdi nil
+ "A cached LSP TextDocumentIdentifier URI string.")
+
(cl-defmethod eglot-handle-notification
(_server (_method (eql textDocument/publishDiagnostics)) &key uri diagnostics
&allow-other-keys) ; FIXME: doesn't respect `eglot-strict-mode'
@@ -2391,9 +2394,14 @@ eglot-handle-notification
((= sev 2) 'eglot-warning)
(t 'eglot-note)))
(mess (source code message)
- (concat source (and code (format " [%s]" code)) ": " message)))
+ (concat source (and code (format " [%s]" code)) ": " message))
+ (find-it (uri)
+ (cl-loop for b in (buffer-list)
+ when (with-current-buffer b
+ (equal eglot--cached-tdi uri))
+ return b)))
(if-let* ((path (expand-file-name (eglot-uri-to-path uri)))
- (buffer (find-buffer-visiting path)))
+ (buffer (find-it uri)))
(with-current-buffer buffer
(cl-loop
initially
@@ -2518,9 +2526,6 @@ eglot-handle-request
(t (setq success :json-false)))
`(:success ,success)))
-(defvar-local eglot--cached-tdi nil
- "A cached LSP TextDocumentIdentifier URI string.")
-
(defun eglot--TextDocumentIdentifier ()
"Compute TextDocumentIdentifier object for current buffer."
`(:uri ,(or eglot--cached-tdi