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

Previous Next

Full log

Message #89 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>,
 Felician Nemeth <felician.nemeth <at> gmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 70036 <at> debbugs.gnu.org
Subject: bug#70036 a fix that
Date: Thu, 18 Apr 2024 16:32:33 +0100

So I've read up on the bug report and I had a close look at the Eglot
usage profiles (not the micro-benchmarks, those are reasonably
irrelevant in what concerns Eglot).  I see these kinds of things in
Theodor's profiles

          18   8%           - eglot--TextDocumentPositionParams
          18   8%            - eglot--TextDocumentIdentifier
          18   8%             - eglot--path-to-uri
          15   7%              - file-truename
          14   6%               - file-truename
          14   6%                - file-truename
          11   5%                 - file-truename
          11   5%                  - file-truename
          11   5%                   - file-truename
          10   4%                    - file-truename
          10   4%                     - file-truename
           8   3%                      - file-truename
           8   3%                       - file-truename
           8   3%                        - file-truename
           5   2%                         - file-truename
           3   1%                          - file-truename
           2   0%                           - file-truename
           1   0%                              file-truename


I could reproduce this, but never even close to the amount of ~7-8%.
Best I could get was 1% and I had to work pretty hard for it.  If I
invoke completion or something heavier like that, it completely
dominates the profile.

          25   1%         - eglot--TextDocumentPositionParams
          23   1%          - eglot--TextDocumentIdentifier
          23   1%           - eglot-path-to-uri
          13   0%            - file-truename
          13   0%             - file-truename
          13   0%              - file-truename
          13   0%                 file-truename

Maybe that's because file-truename is a recursive function that becomes
slower as the path it's asked to analyse becomes longer (obviously,
there can be a symlink at every junction).

So let's assume for the sake of argument that it's frequent for
users to have very long file names with very many components
or that the 1% overhead on the average path length is a real
performance problem for people.

If so, I think this simpler patch after my sig is all we need, as it
completely clears the profile of any "file-truename".  I have reverted
the earlier patch and pushed a patch very similar to the one after my sig.

I'm interested in more details of exactly what constitutes actual
usage, i.e. what did you do while recording a cpu or mem profile.

As to the symlink-outside-the-project issue that Felicián raised,
I was not aware of that bug.  It seems a separate bug at any rate.

I've also noticed that the Eglot automated tests are failing.
That too is unexpected.  I will have a look at that, too.

João


commit 23d6517b2499089f775640b88958774ac2c91049
Author: João Távora <joaotavora <at> gmail.com>
Date:   Thu Apr 18 08:03:10 2024 -0500

    Better way to fix bug#70036

    Cache eglot--cached-tdi per buffer, like we do with some many
    other variables already, to avoid frequent expensive
    recomputation of a value that requires potentially many
    'file-truename' calls.

diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el
index 806265544d6..6196aaff7a3 100644
--- a/lisp/progmodes/eglot.el
+++ b/lisp/progmodes/eglot.el
@@ -2517,12 +2517,17 @@ 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 ,(eglot-path-to-uri (or buffer-file-name
-                                  (ignore-errors
-                                    (buffer-file-name
-                                     (buffer-base-buffer)))))))
+  `(:uri ,(or eglot--cached-tdi
+              (setq eglot--cached-tdi
+                    (eglot-path-to-uri (or buffer-file-name
+                                           (ignore-errors
+                                             (buffer-file-name
+                                              (buffer-base-buffer)))))))))

 (defvar-local eglot--versioned-identifier 0)

-- 
João Távora

Previous Next

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