[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#70036: a fix that
From: |
João Távora |
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@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
- bug#70036: a fix that,
João Távora <=
- bug#70036: a fix that, João Távora, 2024/04/18
- bug#70036: a fix that, Ihor Radchenko, 2024/04/18
- bug#70036: a fix that, Eli Zaretskii, 2024/04/18
- bug#70036: a fix that, Theodor Thornhill, 2024/04/18
- bug#70036: a fix that, João Távora, 2024/04/18
- bug#70036: a fix that, Eli Zaretskii, 2024/04/18
- bug#70036: a fix that, João Távora, 2024/04/18
- bug#70036: a fix that, Eli Zaretskii, 2024/04/18
- bug#70036: a fix that, João Távora, 2024/04/18