bug#70036: 30.0.50; Move file-truename to the C level

[Theodor Thornhill via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>]

[-- Attachment #1: Type: text/plain, Size: 1188 bytes --]

Felician Nemeth <felician.nemeth@gmail.com> writes:

> Theodor Thornhill <theo@thornhill.no> writes:
>
>> This wouldn't help for the usage in find-buffer-visiting, though. But
>> this one could more easily be replaced by reworking the diagnostics
>> handler. We could store the last received diagnostics in the server
>> object, and do a quick lookup from known buffers there.
>
> eglot-handle-notification:textDocument/publishDiagnostics,
> eglot--xref-make-match, and eglot--apply-workspace-edit call
> find-buffer-visiting.  It seems to me only the first case might be
> really time sensitive.  Theo, can you email me the relevant messages
> that your server sends to Emacs?  Does the server send lots of similar
> diagnostics messages frequently?
>
> Thanks,
> Felicián

I'll try to include such a report a little later today. But yes, it
happens a lot. Consider the following patch, though. It eliminates the
issues for publishDiagnostics. It can easily be extended to the other
places where find-buffer-visiting is used.

The publishDiagnostics is sent on change from the server, so that isn't
directly initiated by Eglot.

What do you think?

Theo


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Don-t-use-file-truepath-in-Eglot-bug-70036.patch --]
[-- Type: text/x-diff, Size: 5902 bytes --]

From d43fe85eae05baa07dd4c3e873a1b94542d97ea9 Mon Sep 17 00:00:00 2001
From: Theodor Thornhill <theo@thornhill.no>
Date: Thu, 28 Mar 2024 12:56:23 +0100
Subject: [PATCH] Don't use file-truepath in Eglot (bug#70036)

`file-truepath' is slow because of recursive calls and being implemented
in lisp.  It seems to not be needed in eglot, but it is used behind the
scenes in `find-buffer-visiting', thus appearing in profiles.  Moving
the implementation to a hash map will yield similar performance
benefits, but wouldn't require us to rewrite `file-truename' in C.

* lisp/progmodes/eglot.el (eglot-lsp-server): Convert 'managed-buffers'
to a hashmap.
(eglot-uri-to-path): Don't use file-truepath, as it is too slow to be
included in the hot path.
(eglot--on-shutdown): Use buffers from buffer map.
(eglot--managed-mode): Add buffer to map, rather than list. Also remove
it from the map on deactivation.
(eglot-handle-notification): Expose server and get buffer from the
buffer map.
---
 lisp/progmodes/eglot.el | 30 +++++++++++++++++-------------
 1 file changed, 17 insertions(+), 13 deletions(-)

diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el
index 7d2f1a55165..c6dbca6fc6a 100644
--- a/lisp/progmodes/eglot.el
+++ b/lisp/progmodes/eglot.el
@@ -1053,8 +1053,8 @@ eglot-lsp-server
     :documentation "Map (DIR -> (WATCH ID1 ID2...)) for `didChangeWatchedFiles'."
     :initform (make-hash-table :test #'equal) :accessor eglot--file-watches)
    (managed-buffers
-    :initform nil
-    :documentation "List of buffers managed by server."
+    :documentation "Map (PATH -> BUFFER) for buffers managed by server."
+    :initform (make-hash-table :test #'equal)
     :accessor eglot--managed-buffers)
    (saved-initargs
     :documentation "Saved initargs for reconnection purposes."
@@ -1085,12 +1085,12 @@ eglot-uri-to-path
 
 (defun eglot-path-to-uri (path)
   "Convert PATH, a file name, to LSP URI string and return it."
-  (let ((truepath (file-truename path)))
+  (let ((expanded-path (expand-file-name path)))
     (if (and (url-type (url-generic-parse-url path))
              ;; It might be MS Windows path which includes a drive
              ;; letter that looks like a URL scheme (bug#59338)
              (not (and (eq system-type 'windows-nt)
-                       (file-name-absolute-p truepath))))
+                       (file-name-absolute-p expanded-path))))
         ;; Path is already a URI, so forward it to the LSP server
         ;; untouched.  The server should be able to handle it, since
         ;; it provided this URI to clients in the first place.
@@ -1098,11 +1098,11 @@ eglot-path-to-uri
       (concat "file://"
               ;; Add a leading "/" for local MS Windows-style paths.
               (if (and (eq system-type 'windows-nt)
-                       (not (file-remote-p truepath)))
+                       (not (file-remote-p expanded-path)))
                   "/")
               (url-hexify-string
                ;; Again watch out for trampy paths.
-               (directory-file-name (file-local-name truepath))
+               (directory-file-name (file-local-name expanded-path))
                eglot--uri-path-allowed-chars)))))
 
 (defun eglot-range-region (range &optional markers)
@@ -1187,7 +1187,7 @@ eglot--servers-by-xrefed-file
 (defun eglot--on-shutdown (server)
   "Called by jsonrpc.el when SERVER is already dead."
   ;; Turn off `eglot--managed-mode' where appropriate.
-  (dolist (buffer (eglot--managed-buffers server))
+  (dolist (buffer (map-values (eglot--managed-buffers server)))
     (let (;; Avoid duplicate shutdowns (github#389)
           (eglot-autoshutdown nil))
       (eglot--when-live-buffer buffer (eglot--managed-mode-off))))
@@ -1992,7 +1992,11 @@ eglot--managed-mode
       (add-hook 'eldoc-documentation-functions #'eglot-signature-eldoc-function
                 nil t)
       (eldoc-mode 1))
-    (cl-pushnew (current-buffer) (eglot--managed-buffers (eglot-current-server))))
+
+    (let ((buffer (current-buffer)))
+      (puthash (expand-file-name (buffer-file-name buffer))
+               buffer
+               (eglot--managed-buffers (eglot-current-server)))))
    (t
     (remove-hook 'after-change-functions #'eglot--after-change t)
     (remove-hook 'before-change-functions #'eglot--before-change t)
@@ -2020,10 +2024,10 @@ eglot--managed-mode
     (let ((server eglot--cached-server))
       (setq eglot--cached-server nil)
       (when server
-        (setf (eglot--managed-buffers server)
-              (delq (current-buffer) (eglot--managed-buffers server)))
+        (remhash (expand-file-name (buffer-file-name (current-buffer)))
+                 (eglot--managed-buffers server))
         (when (and eglot-autoshutdown
-                   (null (eglot--managed-buffers server)))
+                   (null (map-values (eglot--managed-buffers server))))
           (eglot-shutdown server)))))))
 
 (defun eglot--managed-mode-off ()
@@ -2346,7 +2350,7 @@ eglot-handle-notification
                           (remhash token (eglot--progress-reporters server))))))))))
 
 (cl-defmethod eglot-handle-notification
-  (_server (_method (eql textDocument/publishDiagnostics)) &key uri diagnostics
+  (server (_method (eql textDocument/publishDiagnostics)) &key uri diagnostics
            &allow-other-keys) ; FIXME: doesn't respect `eglot-strict-mode'
   "Handle notification publishDiagnostics."
   (cl-flet ((eglot--diag-type (sev)
@@ -2357,7 +2361,7 @@ eglot-handle-notification
             (mess (source code message)
               (concat source (and code (format " [%s]" code)) ": " message)))
     (if-let* ((path (expand-file-name (eglot-uri-to-path uri)))
-              (buffer (find-buffer-visiting path)))
+              (buffer (gethash path (eglot--managed-buffers server))))
         (with-current-buffer buffer
           (cl-loop
            initially
-- 
2.40.1