excessive delays inside which-key--maybe-replace when using lsp layer #12455

randomphrase · 2019-06-21T01:30:11Z

Description

I have observed performance degradation in which-key after a day's worth of development with spacemacs. Specifically, pressing the leader key and waiting for the menu to appear takes longer and longer.

Using the emacs profiler, I determined that almost all cpu time was being spent inside which-key--maybe-replace. Investigating further, it seems that the delays are due to growth of the which-key-replacement-alist variable, and this is in turn due to the lsp layer.

In the lsp layer, there is a call to (spacemacs//lsp-declare-prefixes-for-mode major-mode) as a hook attached to lsp-after-open-hook. This adds to the which-key-replacement-alist variable. Growth of this variable is correlated with the delays in showing the spacemacs menu.

Reproduction guide 🪲

Start Emacs
Open a file which uses LSP mode. I believe this could be any major-mode, but I am using c++-mode.
Use SPC b R to revert this buffer
Repeat the above step many times, or just open other files in the same LSP workspace
Press leader key SPC and wait for menu to appear

Observed behaviour: 👀 💔

The more files opened, or the more times each buffer is reverted, the longer it takes to the menu to appear. This is observed to be correlated with the increasing value of (length (assoc major-mode which-key-replacement-alist)).

Expected behaviour: ❤️ 😄

The menu appears instantateously. Also the value of (length (assoc major-mode which-key-replacement-alist)) is stable.

System Info 💻

OS: gnu/linux
Emacs: 26.1
Spacemacs: 0.300.0
Spacemacs branch: develop (rev. 37dcecb)
Graphic display: nil
Distribution: spacemacs
Editing style: emacs
Completion: helm
Layers:

(systemd javascript html sql auto-completion better-defaults
         (c-c++ :variables c-c++-backend 'lsp-ccls c-c++-lsp-executable
                (expand-file-name "~/hack/extern/ccls/Release/ccls"))
         clojure dap
         (dash :variables helm-dash-docset-newpath
               (expand-file-name "~/.local/share/Zeal/Zeal/docsets"))
         emacs-lisp ess git helm markdown
         (org :variables org-enable-reveal-js-support t)
         python shell-scripts syntax-checking version-control)

System configuration features: XPM JPEG TIFF GIF PNG RSVG IMAGEMAGICK SOUND GPM DBUS GSETTINGS NOTIFY ACL LIBSELINUX GNUTLS LIBXML2 FREETYPE M17N_FLT LIBOTF XFT ZLIB TOOLKIT_SCROLL_BARS GTK3 X11 THREADS LIBSYSTEMD LCMS2

Backtrace 🐾

The text was updated successfully, but these errors were encountered:

thanhvg · 2019-06-24T06:13:17Z

Your investigation is awesome. I can also reproduce this bug with js layer.

add an internal variable for `lsp` layer `lsp-layer--active-mode-list` to keep track of active major modes using lsp, `spacemacs//lsp-declare-prefixes-for-mode` will check this variable to decide whether to add prefix to which-key or not

vv111y · 2019-08-06T20:47:40Z

commit 653a38b doesn't seem to work. I am still experiencing this.
@randomphrase great stuff digging into this. It is indeed in which-key--maybe-replace, and my which-key-replacement-alist was over 175K.

thanhvg · 2019-08-06T21:06:56Z

commit 653a38b doesn't seem to work. I am still experiencing this.
@randomphrase great stuff digging into this. It is indeed in which-key--maybe-replace, and my which-key-replacement-alist was over 175K.

@vv111y can you describe how to reproduce this bug in your case? thanks

randomphrase · 2019-08-11T15:50:45Z

This does seem to be fixed for me. Thanks @thanhvg!

add an internal variable for `lsp` layer `lsp-layer--active-mode-list` to keep track of active major modes using lsp, `spacemacs//lsp-declare-prefixes-for-mode` will check this variable to decide whether to add prefix to which-key or not

vv111y · 2020-11-07T17:57:55Z

First - my apologies for not responding. I wasn't sure what to report. The correct response would have been: "just use spacemacs".

This issue was never resolved and the relevant information is upstream:
justbur/emacs-which-key#226

Proposed solution:
justbur/emacs-which-key#261
with the relevant comment:
justbur/emacs-which-key#261 (comment)

This requires spacemacs to modify how which-key is used (I don't know what that is, I haven't had the time to learn how this works). Shall I open a new issue? Is there anything I can do to help?

This commit added add a new function defun spacemacs/declare-prefix-for-minor-mode and improved spacemacs/set-leader-keys-for-minor-mode. `which-key` package recently introduced a new api which-key-add-keymap-based-replacements which improves perfomance and allows prefix and namings to be stored directly in keymap. This is a great improvement. With this new api we now make change to spacemacs/declare-prefix-for-minor-mode to manage prefix also. For example: (spacemacs/set-leader-keys-for-minor-mode 'lsp-mode "=" "format" "=b" #'lsp-format-buffer) Before we had to use another api to bind prefix spacemacs/declare-prefix-for-mode which only works on major-mode. As lsp-mode is a minor mode this api causes a lot of problems to which-key performance. An example is syl20bnr#12455 which led to my hack in syl20bnr#12474. The improved spacemacs/set-leader-keys-for-minor-mode will take care of both prefix and key naming for the minor mode. This will allows us to have a better set up for dynamic minor modes such as lsp-mode, tide-mode etc. Also another api is created to make prefix for minor mode: spacemacs/declare-prefix-for-minor-mode. Usage: (spacemacs/declare-prefix-for-minor-mode 'tide-mode "E" "errors")"

…d apply it to tide and lsp layers (#14141) * [core][keybinng] improve minor mode binding This commit added add a new function defun spacemacs/declare-prefix-for-minor-mode and improved spacemacs/set-leader-keys-for-minor-mode. `which-key` package recently introduced a new api which-key-add-keymap-based-replacements which improves perfomance and allows prefix and namings to be stored directly in keymap. This is a great improvement. With this new api we now make change to spacemacs/declare-prefix-for-minor-mode to manage prefix also. For example: (spacemacs/set-leader-keys-for-minor-mode 'lsp-mode "=" "format" "=b" #'lsp-format-buffer) Before we had to use another api to bind prefix spacemacs/declare-prefix-for-mode which only works on major-mode. As lsp-mode is a minor mode this api causes a lot of problems to which-key performance. An example is #12455 which led to my hack in #12474. The improved spacemacs/set-leader-keys-for-minor-mode will take care of both prefix and key naming for the minor mode. This will allows us to have a better set up for dynamic minor modes such as lsp-mode, tide-mode etc. Also another api is created to make prefix for minor mode: spacemacs/declare-prefix-for-minor-mode. Usage: (spacemacs/declare-prefix-for-minor-mode 'tide-mode "E" "errors")" * [tide] improve prefix * [lsp] improve prefix

opsxcq · 2020-12-14T13:29:58Z

Observing a similar issue, but also which-keu--create-buffer-and-show creating a huge delay for everything (even save)

- timer-event-handler                                           10186  87%
 - apply                                                        10173  87%
  - which-key--update                                            9777  83%
   - which-key--create-buffer-and-show                           9777  83%
    - which-key--get-bindings                                    9569  82%
     - which-key--format-and-replace                             9502  81%
      + which-key--maybe-replace                                 9438  81%

smile13241324 · 2020-12-18T21:52:13Z

Interesting, do you see the same behaviour on latest develop @opsxcq? This should not longer be the case.

…d apply it to tide and lsp layers (syl20bnr#14141) * [core][keybinng] improve minor mode binding This commit added add a new function defun spacemacs/declare-prefix-for-minor-mode and improved spacemacs/set-leader-keys-for-minor-mode. `which-key` package recently introduced a new api which-key-add-keymap-based-replacements which improves perfomance and allows prefix and namings to be stored directly in keymap. This is a great improvement. With this new api we now make change to spacemacs/declare-prefix-for-minor-mode to manage prefix also. For example: (spacemacs/set-leader-keys-for-minor-mode 'lsp-mode "=" "format" "=b" #'lsp-format-buffer) Before we had to use another api to bind prefix spacemacs/declare-prefix-for-mode which only works on major-mode. As lsp-mode is a minor mode this api causes a lot of problems to which-key performance. An example is syl20bnr#12455 which led to my hack in syl20bnr#12474. The improved spacemacs/set-leader-keys-for-minor-mode will take care of both prefix and key naming for the minor mode. This will allows us to have a better set up for dynamic minor modes such as lsp-mode, tide-mode etc. Also another api is created to make prefix for minor mode: spacemacs/declare-prefix-for-minor-mode. Usage: (spacemacs/declare-prefix-for-minor-mode 'tide-mode "E" "errors")" * [tide] improve prefix * [lsp] improve prefix

opsxcq · 2021-03-18T20:01:41Z

I will test with the latest develop and validate

opsxcq · 2021-03-18T20:21:24Z

@smile13241324 no, got the last version from develop and the result still the same. But apparently it gets worse when I use org mode, babel and sessions

- timer-event-handler                                           27043  84%
 - apply                                                        27043  84%
  - which-key--update                                           26748  84%
   - which-key--create-buffer-and-show                          26748  84%
    + which-key--get-bindings                                   25279  79%
    + which-key--create-pages                                    1441   4%
    + which-key--show-page                                         28   0%
  + #<compiled 0x1ffc3f>                                          197   0%
  + savehist-autosave                                              15   0%
  + sp-show--pair-function                                         11   0%
  + auto-revert-buffers                                             7   0%
- command-execute                                                3665  11%
 - call-interactively                                            3665  11%
  - funcall-interactively                                        3665  11%
   + lazy-helm/helm-mini                                         3174   9%
   + spacemacs/helm-M-x-fuzzy-matching                            430   1%
   + comint-send-input                                             55   0%
   + evil-normal-state                                              6   0%

vv111y · 2021-03-18T21:16:55Z

fwiw the situation has improved for me except now it's helm-descbinds.
I timed it and it takes 1 minute 23 seconds to pop up. I just don't use it anymore.
Doesn't seem to be related, but just in case I'll post it.

- command-execute                                               42598  98%
 - funcall-interactively                                        42598  98%
  - helm-descbinds                                              42597  98%
   - helm                                                       33277  77%
    - helm                                                      33277  77%
     - helm-internal                                            33277  77%
      - helm-read-from-minibuffer                               33237  77%
       - helm-update                                            33085  76%
        - helm--collect-matches                                 33071  76%
         - helm-compute-matches                                 33071  76%
          - helm-process-filtered-candidate-transformer              33065  76%
           - helm-apply-functions-from-source                   33065  76%
            - helm-fuzzy-highlight-matches                      33058  76%
             - helm-flx-fuzzy-highlight-match                   33034  76%
              - require                                         32051  74%
                 apply                                          32048  74%
              + helm-flx-fuzzy-highligher                         876   2%
          + helm-get-cached-candidates                              6   0%
        + helm-render-source                                       12   0%
        + #<compiled -0x1d0af811aa49e27f>                           1   0%
        + helm--update-move-first-line                              1   0%
       + timer-event-handler                                       58   0%
       + command-execute                                           22   0%
       + redisplay_internal (C function)                            7   0%
         helm-get-candidate-number                                  3   0%
       + substitute-command-keys                                    1   0%
      + #<subr F616e6f6e796d6f75732d6c616d626461_anonymous_lambda_84>                 14   0%
      + helm-initialize                                             1   0%
      + helm-display-buffer                                         1   0%
   - helm-descbinds-sources                                      9320  21%
    - helm-descbinds-all-sections                                9301  21%
     - describe-buffer-bindings                                  9278  21%
      - apply                                                    9278  21%
       - #<subr describe-buffer-bindings>                        9278  21%
        - describe-map-tree                                      9272  21%
         + describe-map                                            44   0%
       replace-regexp-in-string                                     2   0%
    + #<subr F616e6f6e796d6f75732d6c616d626461_anonymous_lambda_11>                  7   0%
+ timer-event-handler                                             217   0%
+ ...                                                             208   0%
+ redisplay_internal (C function)                                  33   0%
+ which-key--hide-popup                                             2   0%

…d apply it to tide and lsp layers (syl20bnr#14141) * [core][keybinng] improve minor mode binding This commit added add a new function defun spacemacs/declare-prefix-for-minor-mode and improved spacemacs/set-leader-keys-for-minor-mode. `which-key` package recently introduced a new api which-key-add-keymap-based-replacements which improves perfomance and allows prefix and namings to be stored directly in keymap. This is a great improvement. With this new api we now make change to spacemacs/declare-prefix-for-minor-mode to manage prefix also. For example: (spacemacs/set-leader-keys-for-minor-mode 'lsp-mode "=" "format" "=b" #'lsp-format-buffer) Before we had to use another api to bind prefix spacemacs/declare-prefix-for-mode which only works on major-mode. As lsp-mode is a minor mode this api causes a lot of problems to which-key performance. An example is syl20bnr#12455 which led to my hack in syl20bnr#12474. The improved spacemacs/set-leader-keys-for-minor-mode will take care of both prefix and key naming for the minor mode. This will allows us to have a better set up for dynamic minor modes such as lsp-mode, tide-mode etc. Also another api is created to make prefix for minor mode: spacemacs/declare-prefix-for-minor-mode. Usage: (spacemacs/declare-prefix-for-minor-mode 'tide-mode "E" "errors")" * [tide] improve prefix * [lsp] improve prefix

duianto added the - Bug tracker - label Jun 24, 2019

thanhvg mentioned this issue Jun 25, 2019

fix 12455 lsp layer #12474

Closed

randomphrase closed this as completed Aug 11, 2019

peterhoeg mentioned this issue Nov 26, 2019

Lots of time spent inside which-key--maybe-replace justbur/emacs-which-key#226

Open

smile13241324 mentioned this issue Mar 2, 2020

[feature] tide layer #13305

Closed

thanhvg mentioned this issue Nov 17, 2020

[core][tide][lsp] improve spacemacs/set-leader-keys-for-minor-mode and apply it to tide and lsp layers #14141

Merged

thanhvg mentioned this issue May 3, 2021

[lsp] add option to bind commands to lsp-command-map #14425

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

excessive delays inside which-key--maybe-replace when using lsp layer #12455

excessive delays inside which-key--maybe-replace when using lsp layer #12455

randomphrase commented Jun 21, 2019

thanhvg commented Jun 24, 2019

vv111y commented Aug 6, 2019 •

edited

Loading

thanhvg commented Aug 6, 2019

randomphrase commented Aug 11, 2019

vv111y commented Nov 7, 2020

opsxcq commented Dec 14, 2020 •

edited

Loading

smile13241324 commented Dec 18, 2020

opsxcq commented Mar 18, 2021

opsxcq commented Mar 18, 2021

vv111y commented Mar 18, 2021

excessive delays inside which-key--maybe-replace when using lsp layer #12455

excessive delays inside which-key--maybe-replace when using lsp layer #12455

Comments

randomphrase commented Jun 21, 2019

Description

Reproduction guide 🪲

System Info 💻

Backtrace 🐾

thanhvg commented Jun 24, 2019

vv111y commented Aug 6, 2019 • edited Loading

thanhvg commented Aug 6, 2019

randomphrase commented Aug 11, 2019

vv111y commented Nov 7, 2020

opsxcq commented Dec 14, 2020 • edited Loading

smile13241324 commented Dec 18, 2020

opsxcq commented Mar 18, 2021

opsxcq commented Mar 18, 2021

vv111y commented Mar 18, 2021

vv111y commented Aug 6, 2019 •

edited

Loading

opsxcq commented Dec 14, 2020 •

edited

Loading