GNU bug report logs - #70036
30.0.50; Move file-truename to the C level

Previous Next

Full log

Message #155 received at 70036 <at> debbugs.gnu.org (full text, mbox):

From: João Távora <joaotavora <at> gmail.com>
To: Theodor Thornhill <theo <at> thornhill.no>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 70036 <at> debbugs.gnu.org,
 felician.nemeth <at> gmail.com
Subject: Re: bug#70036 a fix that
Date: Fri, 19 Apr 2024 00:59:49 +0100

[Message part 1 (text/plain, inline)]

On Thu, Apr 18, 2024 at 11:06 PM João Távora <joaotavora <at> 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

[Message part 2 (text/html, inline)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.