Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Slowness when using for Clojure #39

Open
ericdallo opened this issue Mar 27, 2023 · 12 comments
Open

Slowness when using for Clojure #39

ericdallo opened this issue Mar 27, 2023 · 12 comments

Comments

@ericdallo
Copy link

Hi, I really like consult-lsp-symbols but I can't use it with most projects I work, even medium/small projects because of Emacs freezing and taking too long to return the symbols.

One good repo to test is the clojure-lsp repo itself and if we check server logs we can see the server is pretty fast
image

I wonder if there is any quick wins for performance in consult-lsp side

@gagbo
Copy link
Owner

gagbo commented Mar 27, 2023

It's really weird that it freezes Emacs as it's literally the only thing that uses the async facilities of consult.

Did you try playing with the consult knobs around async, namely

(setq consult-async-refresh-delay 0.01
      consult-async-input-throttle 0.01
      consult-async-input-debounce 0.01
      consult-async-min-input 1)

@ericdallo
Copy link
Author

Thanks, I'm using doom-emacs default config, but just I tried those config but still the same issue.
One thing I noticed is that the freeze happens even typing one single char, but it's weird because consult-async-min-input that doom's configure is 2, I'd expect to only do the query after typing 2 chars

@ericdallo
Copy link
Author

I also tried toggle-debug-on-quit and type C-g when Emacs freeze and this is the stack I get:
image

maybe it's something with vertico?

@agzam
Copy link

agzam commented Mar 27, 2023

I confirm @ericdallo's findings. This feature is slow even in a moderately sized Clojure project - ~43K LOC. I'm using the latest Emacs with --native-comp flag. The initial call of the command takes about a second or two, which is fine - it needs to gather data and whatnot. But then, typing for a symbol freezes it up for every character I type. @gagbo, if you could guide us on how to debug this, it would be awesome. I want to use this feature a lot, but this minor annoyance keeps me away.

@gagbo
Copy link
Owner

gagbo commented Mar 27, 2023

Even the part where it takes a second or two when loading shouldn't happen. What consult-lsp-symbols is meant to do is:

I guess there's no debouncing mechanism in the current version.

So realistically, the only reason you should have the initial call being long is when invoking consult-lsp-symbols triggered autoloading of consult-lsp, which maybe loaded cl-lib that wasn't loaded before? Otherwise, at least from my understanding of consult API, there's no reason that going from "M-x consult-lsp-symbols RET" to "a minibuffer waiting for input" takes more than basically vertico--setup duration.

Do you use plists with lsp-mode or not? Do you have a way to see whether a lot of garbage collections get triggered? I don't know yet when I have time to dig into installing clojure and test stuff on the clojure-lsp repo. But maybe there's a lot of GC being triggered from parsing the content of the response and then calling the map operator on the list?

But I couldn't follow the breaking changes in consult since I first wrote it (and I had no choice but to use the consult-- functions because there's no way to implement a command otherwise), so maybe I missed something that triggered a regression

@agzam
Copy link

agzam commented Mar 27, 2023

Even the part where it takes a second or two when loading shouldn't happen

You know what? I just did some re-installing, updated clojure-lsp server, updated packages, etc. And now consult-lsp-symbols is working without the problems I had before. Yet it weirdly doesn't work at all in another project. But lsp-ido-workspace-symbol not working in that project either. So it seems to be a completely unrelated problem. Let me do some more digging, but it already looks like you said: it's not a problem with this package; it's somewhere else.

Thank you for your help and your time @gagbo!

@ericdallo
Copy link
Author

I guess I'll have to do the same @agzam 😅

@aisamu
Copy link
Contributor

aisamu commented Apr 27, 2024

👋🏻

I ran into this today and I might have found something relevant.
There appears to be a transformer doing an expensive/slow operation on the results:

  2223  84% - command-execute
  2223  84%  - funcall-interactively
  2220  84%   - consult-lsp-symbols
  2220  84%    - let*
  2220  84%     - consult--read
  2220  84%      - consult--read-1
  2220  84%       - consult--with-preview-1
  2220  84%        - #<compiled -0x58325301850aab9>
  2220  84%         - completing-read
  2220  84%          - completing-read-default
  2220  84%           - apply
  2220  84%            - vertico--advice
  2220  84%             - apply
  2220  84%              - #<compiled -0x1ca6dd56b0e8db67>
  2220  84%               - apply
  2202  84%                - #<subr completing-read-default>
  2180  83%                 - #<compiled -0x780f8018c142a44>
  2149  82%                  - mapc
  2149  82%                   - #<compiled 0xebc250b63c1dfab>
  2149  82%                    - lsp--parser-on-message
  2149  82%                     - #<compiled 0xaec9ae6f9f1fea5>
  2149  82%                      - #<compiled -0xd8c37fcffb7c3cd>
  1102  42%                       - #<lambda 0x1e1e0bec3caaad15>
  1102  42%                        - funcall
  1102  42%                         - #<compiled -0x1d1d86cf42d3ec91>
  1102  42%                          - mapcar
  1102  42%                           - consult-lsp--symbols--transformer
  1101  42%                            - propertize
  1101  42%                             - format
+ 1101  42%                              - consult-lsp--format-file-line-match           <= Start =>
+ 1100  42%                               - let
+ 1099  41%                                - let*
+ 1040  39%                                 - and
+ 1039  39%                                  - lsp-workspace-root
+ 1032  39%                                   - lsp-f-ancestor-of?
+ 1014  38%                                      lsp-f-same?                              <= End =>
     6   0%                                     lsp--files-same-host
    59   2%                                 + if
     1   0%                                + lsp--uri-to-path
  1047  39%                       - #<lambda 0x1e1e0bec3caaad15>
  1047  39%                        - funcall
  1047  39%                         - #<compiled -0x1d1d86cf42d3ec91>
  1046  39%                          - mapcar
  1046  39%                           - consult-lsp--symbols--transformer
  1043  39%                            - propertize
  1042  39%                             - format
  1042  39%                              - consult-lsp--format-file-line-match
  1041  39%                               - let
  1035  39%                                - let*
   982  37%                                 - and
   982  37%                                  - lsp-workspace-root
   975  37%                                   - lsp-f-ancestor-of?
   953  36%                                      lsp-f-same?
    53   2%                                   if
     5   0%                                + lsp--uri-to-path
     1   0%                          + #<compiled 0xefb4f0b9e249741>

Removing that query from the transform path makes it behave like other async consults (e.g. ripgrep) - I can type normally (no jankyness) and results show up a little bit later (with their absolute paths, but still):

diff --git a/consult-lsp.el b/consult-lsp.el
index 1fc0de2..48e45f4 100644
--- a/consult-lsp.el
+++ b/consult-lsp.el
@@ -383,9 +383,8 @@ When ARG is set through prefix, query all workspaces."
            (consult-lsp--format-file-line-match
             (let ((file
                    (lsp--uri-to-path (lsp:location-uri (lsp:symbol-information-location symbol-info)))))
-              (if-let ((wks (lsp-workspace-root file)))
-                  (f-relative file wks)
-                file))
+              file
+              )
             (thread-first symbol-info
                           (lsp:symbol-information-location)
                           (lsp:location-range)

Here's the CPU profile after the change

 309  64% - command-execute
 309  64%  - funcall-interactively
 307  63%   - consult-lsp-symbols
 307  63%    - let*
 307  63%     - consult--read
 307  63%      - consult--read-1
 307  63%       - consult--with-preview-1
 307  63%        - #<compiled -0x58325301850aab9>
 307  63%         - completing-read
 307  63%          - completing-read-default
 307  63%           - apply
 307  63%            - vertico--advice
 307  63%             - apply
 307  63%              - #<compiled -0x1ca6dd56b0e8db67>
 307  63%               - apply
 250  51%                - #<subr completing-read-default>
 186  38%                 - #<compiled -0x780f8018c142a44>
 128  26%                  - mapc
 128  26%                   - #<compiled 0xebc250b63c1dfab>
 128  26%                    - lsp--parser-on-message
 128  26%                     - #<compiled 0xaec9ae6f9f0c6a5>
 128  26%                      - #<compiled -0xd8c37fcffb7c3cd>
  42   8%                       - #<lambda 0x1e1e0bec3caaad15>
  42   8%                        - funcall
  42   8%                         - #<compiled -0x1d1d86cf42d3ec91>
  40   8%                          - mapcar
+ 40   8%                           - consult-lsp--symbols--transformer
+ 39   8%                            - propertize
+ 37   7%                             - format
+ 37   7%                              - consult-lsp--format-file-line-match

The feature is very useful - absolute paths are noisy/redundant - so I wonder if there's a way to keep it!
I'm not sure if it's fixable - I can see that the transformation is already part of the async "pipeline", and even then it locks up the UI.
Perhaps we could officially ask LSP to return relative paths to begin with? (It doesn't look like it given the spec)

@gagbo
Copy link
Owner

gagbo commented Apr 29, 2024

Yeah, we always need to keep the absolute paths, because otherwise we’ll need a lot of code to disambiguate results from different workspaces I feel. Thanks a lot for the report it’s very insightful!!!

Maybe the solution would be to cache the list of known workspace roots in a global consult-lsp hashmap or prefix-tree of some sort, so that the lookup is as fast as possible.

Haven’t watched your solution in the draft PR yet, it might just be better

@aisamu
Copy link
Contributor

aisamu commented May 8, 2024

@ericdallo @agzam Do you mind giving main (6858391) another try?

@agzam
Copy link

agzam commented May 8, 2024

@aisamu I saw the update and immediately got excited. I'm going to need to find a massive project to fully test it. For a project of moderate size, it works quite well. Very nice. I thank you for this gift.

@aisamu
Copy link
Contributor

aisamu commented May 8, 2024

That's great to hear 🥳!
If you're using vertico and orderless, please also give the "I'm feeling lucky" pattern a try: use ##query instead of #query.
clojure-lsp sends all the symbols on the first call, which we can immediately filter/narrow using using regular emacs/orderless machinery (purely client-side)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants