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

Optimizing for speed: do's and don'ts #502

Closed
blaenk opened this issue Mar 29, 2022 · 9 comments · Fixed by #503
Closed

Optimizing for speed: do's and don'ts #502

blaenk opened this issue Mar 29, 2022 · 9 comments · Fixed by #503

Comments

@blaenk
Copy link

blaenk commented Mar 29, 2022

Hey @noctuid, doubt you remember me but I was a somewhat early adopter (I think?) of your awesome package!

Sorry for the wall of text. I decided to spin this off of #497 because I didn't want to derail your thread.

Lately I've used Emacs less and less for general tasks and increasingly mostly for Magit, so I was going around pruning my configuration of slow/unnecessary packages as much as possible in an effort to make it load as fast as possible with the bare minimum (utils like general.el, evil, magit).

I wasn't planning on getting rid of general.el (and still am not) as I saw it as a crucial bare necessity, but after much trial and error/bisecting I realized that indeed as you say, the mere act of having :general clauses in my use-package definitions, was causing evil to load very substantially slower. (I realize technically that's not necessarily what's causing the slow-down, I'm saying it from a user perspective).

I'd appreciate any help whatsoever that you can provide here short of your rewrite that would allow me to salvage the use of general.el in some form. In general would you be kind enough to explain to us as users in slightly more detail, what exactly are the cases that lead to this degenerate slow behavior, in the hopes that I can avoid them while retaining use of general.el? Or phrased another way: could you maybe share do's/don'ts if we're interested in performance? For example it seems you're saying we should probably avoid ":global-prefix, :non-normal-prefix, and :prefix".

For what it's worth: it really is very noticeably slower for my particular configuration because I have lots of bindings defined. Previously I didn't care since I figure, even if it's slow to start emacs, I'll keep it around and make use of it for a while. Now since my use case has changed, it just feels wrong to me that it take that long to open emacs for a quick Magit session, so I'm interested in cracking down on that if at all possible.

If you like, feel free to respond in a generalized form in the readme and then link me there!

My custom map is defined here:

https://github.com/blaenk/dots/blob/d0389c15ab5fb377ed59b868e64a06ae18def195/emacs/.emacs.d/inits/conf/common.el#L32-L36

(general-create-definer my-map
  :states '(emacs normal visual insert)
  :global-prefix "C-c"
  :non-normal-prefix "M-SPC"
  :prefix "SPC")

Some questions:

  • Is there something about that definition in particular that subjects me to increased slowness? I guess I have the trifecta of things you're saying cause slowness like :global-prefix. Can you share what makes that slow? If I were to get rid of these settings then I guess it would simply cause the bindings to no longer be "root" level bindings so to speak (I think of the bindings as a DAG), so I'd have to specify the "root" keymaps to attach to?

  • Can you expand on:

    Maybe this can be kept as opt-in to make migration easier, but using with-eval-after-load, use-package's :config, or maybe a newly added :general-config should be recommended instead.

    Are you saying that I could instead not set a :general clause on use-package and move those (my-map …) calls to the :config section. So what I'll lose there is that I will no longer auto-load the package when I press the keys that I define, and instead those keys won't "work" until I cause the package to load in some other way so that the keymap exists?

    Can the same be accomplished by disabling autoloaded keymaps (assuming that is what's slow to do), so I set general-use-package-emit-autoloads off and then I can keep the :general clauses but they simply won't work until the keymap exists, or is it the case that since the keymap doesn't exist at the point of evaluation then those keybinds won't work even after the keymap exists, since the binding code won't have run again?

    If that's the case, if I'm understanding correctly, do you think a possible compromise to both keeping the top-level :general clauses (there's just something nice about seeing them upfront, it feels self-documenting in a way, but I can live without them if this isn't possible!) while ensuring that the keymaps will work once the package/keymap eventually exists, would be to add a new :on-config t style keyword similar to how you have :no-autoload t mentioned here? Its behavior would basically cause the forms in that section to be evaluated only after :config. I guess at that point it may seem, why even bother, why not just move everything to :config. I just feel like it's super clean to have binds upfront in a self-documenting way, distinct from actual configuration behavior, even if it is a form of configuration, this way it doesn't make the config section too noisy. But if it's too much trouble no worries at all, I'll live!

    I guess at that point if I'm doing it everywhere, I would set general-use-package-emit-autoloads and whatever new global flag you'd create for this.

    EDIT: I realize now that this is probably what you meant by a :general-config section in use-package. That works too, whichever is easiest, if you can do it, it would be immensely appreciated!!

More succinctly: what could I do, even if it heavily reduces ergonomics (e.g. it becomes more verbose/explicit), to recover the speed cost? How should I go about making bindings if I got rid of the prefix settings instead?

@noctuid
Copy link
Owner

noctuid commented Mar 29, 2022

Hey @noctuid, doubt you remember me but I was a somewhat early adopter (I think?) of your awesome package!

No, I remember you :D

In general would you be kind enough to explain to us as users in slightly more detail, what exactly are the cases that lead to this degenerate slow behavior

There are two main issues:

  • Using the deferred keybinding functionality (i.e. binding in a keymap that does not yet exist)
  • Using :global-prefix and :non-normal-prefix (the code that generates/handles prefix keybindings is slow)

The first issues is probably more severe. evil-delay is very slow.

To improve speed

  • You should use with-eval-after-load, general-after or use-package's :config instead of binding keys in keymaps that don't yet exist
  • Use :general only for keybindings meant to load the package; put everything else in :config (this is what I do) or a newly added :general-config

The second issue is a bit more tricky. I already rewrote prefix handling to be faster in the prototype I mention in that issue, but that doesn't help anyone until the rewrite is actually complete (or has a minimal feature set). I would address the first issue and then see if Emacs loading is still too slow for you. If you are mainly using Emacs for magit, then not many general forms should need to run on startup anyway, so the prefix issue might not be a problem.

Doom emacs has rewritten some sort of similar prefix handling as a workaround to general's being slow. For now, you could look into copying that code if there is still a problem. You could also look into using the dumper if speed is really that much of an issue still.

Let me know if further clarification is needed.

Action items

  • Document speed issues in readme and link to relevant issues
  • Add :general-config
  • Investigate if there is an easy to fix the prefix speed issue without the rewrite

@noctuid
Copy link
Owner

noctuid commented Mar 29, 2022

I looked at my previous notes. The reason :non-normal-prefix and :global-prefix are slow:

  • General concatenates the prefix (and infix if it exists) to all keybindings; it will do this 3 times instead of 1x if you specify all prefix keywords
  • Using all three prefix keywords triples the number of keybindings made

There is actually a simple fix you can do since you are already using general-create-definer:

  • Only use the prefix keywords once to bind your prefixes to a prefix map
  • Make my-map bind keys once in a prefix map
;; this will bind the prefixes to the prefix map
(general-define-key
 :states '(emacs normal visual insert)
 :prefix-map 'my-prefix-map
 :global-prefix "C-c"
 :non-normal-prefix "M-SPC"
 :prefix "SPC")

(general-create-definer my-map
  :keymaps 'my-prefix-map)

:general-config should be simple to implement. You can try this (untested):

(defalias 'use-package-normalize/:general-config #'use-package-normalize/:general)

(defalias 'use-package-handler/:general-config #'use-package-handler/:general)

;; add :general-config to `use-package-keywords` after :config
(setq use-package-keywords '(.... :config :general-config ...))

@blaenk
Copy link
Author

blaenk commented Mar 29, 2022

Thank you so much for the wealth of information, I really appreciate your time!

I'll give this a shot when I get a chance and report back.

@blaenk
Copy link
Author

blaenk commented Apr 2, 2022

So I had a chance to apply some the ideas you gave me, hopefully correctly. I'm sure with the change to globally defer packages in use-package, not emit autoloads in general, etc., I probably have some lingering package(s) that aren't configured correctly in that they're not loading when I expect them to, I already know I have a few evil packages like this, but I think I can fix those over time.

Overall your changes have indeed yielded noticeable results and I've been able to arrive at my goal config consisting of basic utilities, evil, and primarily magit—at least the first pass which I can continue refining/pruning from there. The :general-config part appears to have worked, too.

Thank you so much for your work on general and your helpful advice!

Certainly feel free to close this if you wish, or use it as an ongoing performance-addressing issue, up to you! Either way, if others are pursuing something similar, they may benefit from the information you provided so thanks again for that.


If anyone else comes here and is curious what I did, you can find the relevant commits (and extra ones as well, as I was doing general cleaning) in this commit range.

Feel free to let me know if you spot glaring issues!

What I primarily did was as @noctuid mentioned:

  • I added a new :general-config keyword for use-package that behaves like :general but runs after :config.

    ;; Alias :general-config to :general
    (defalias 'use-package-normalize/:general-config #'use-package-normalize/:general)
    (defalias 'use-package-handler/:general-config #'use-package-handler/:general)
    
    ;; Add :general-config to `use-package-keywords` after :config, so that it processes
    ;; after the package has loaded.
    (setq use-package-keywords
      (-insert-at (+ 1 (-elem-index :config use-package-keywords))
        :general-config use-package-keywords))
  • I redefined my general definer to instead add to a custom prefix map (not sure if I did this correctly but it looks the same as what noctuid provided):

    (general-create-definer my-map
    (general-define-key
      :states '(emacs normal visual insert)
      :prefix-map 'my-prefix-map
      :global-prefix "C-c"
      :non-normal-prefix "M-SPC"
      :prefix "SPC")
    
    (general-create-definer my-map
      :keymaps 'my-prefix-map)
  • I replaced :general with :general-config everywhere, but be careful as this is not technically automatically correct. In particular, any bindings you had in :general that bind onto maps created by the package itself, won't work, since it was :general which would "intercept" those and then autoload the package, then bind them to the loaded keymap. You'll need to find some other way to load the package in these situations, but that's more a use-package question (e.g. :defer, or binding to a command that itself is an autoload, etc.)

  • Going further I disabled general's above behavior, where it "intercepts" keys for package keymaps. I'm not sure if this is necessary with judicious use of :general-config. @noctuid: Is it the case that I could use :general-config the majority of the time, and then for the rare few binds that I want to autoload the package, leave those, and it shouldn't impact performance as much? I guess more concretely my question is, does the performance impact scale in proportion to the number of bindings, or is it a hard baseline cost as soon as you do any one of these? Specifically curious across packages, as I imagine that within a single package there is the hard baseline cost of autoloading the package itself when the bind is pressed.

    (eval-and-compile (setq general-use-package-emit-autoloads nil))
  • use-package (non-general): I globally applied :defer to packages so that I don't get accidentally bitten by slow packages loading. Now the reverse problem is true: a package may accidentally not load when I expect it to (because there is nothing to cause it to be loaded, since it's :deferred by default), but I feel like that problem is much easier to address usually since I will notice it missing when I need it (and if I don't need it, maybe it's great then that the package never loaded), at which point I can in a more targeted manner go about fixing that. If you're wondering what that entails, it would be your typical approaches such as: running an autoload from the package (e.g. invoking the mode) on a hook like prog-mode-hook, using :defer to load it after some amount of time, etc.

  • straight.el (non-general): I disabled straight's bootstrapping check, which installs straight.el if it's not installed. Admittedly I did this as part of sweeping changes and maybe it doesn't have as large an impact as I thought, but at the same time I figured that if I ever need to bootstrap, I can invoke it "manually" by uncommenting those lines, letting it bootstrap, then re-commenting them, saving myself the extra syscall(s) in the vast majority of cases since I'll almost always have it installed.

I don't have cold hard numbers unfortunately. I felt I didn't need numbers since this wasn't about shaving away extra milliseconds, but rather a very very noticeable, night and day difference of +5 maybe even +8 seconds compared to now maybe < 2 seconds which is more tolerable. Admittedly either one is "tolerable" and is why I never bothered to really optimize, since I open emacs for what I intend to be long sessions, but more and more lately I find myself opening it for quick sessions almost exclusively limited to magit. The reason I didn't go to the extreme end and just keep magit is because I still would like to have certain utilities like evil, general, which-key, etc.

@noctuid
Copy link
Owner

noctuid commented Apr 3, 2022

I will add :general-config and update the README/FAQ and then close the issue.

I replaced :general with :general-config everywhere, but be careful as this is not technically automatically correct. In particular, any bindings you had in :general that bind onto maps created by the package itself, won't work, since it was :general which would "intercept" those and then autoload the package, then bind them to the loaded keymap. You'll need to find some other way to load the package in these situations, but that's more a use-package question (e.g. :defer, or binding to a command that itself is an autoload, etc.)

does the performance impact scale in proportion to the number of bindings

Yes, it will scale. It's fine to still use :general for keybindings meant to load the package. This is what I do in my config: :general is only used to create keybindings I want available before a package is loaded that should load the package. The difference between using :general and :general-config for a single keybinding won't matter much, but for a large number of keybindings, it will save some time to defer them until the relevant packages load.

It may not save a significant amount of time, but you can keep general-use-package-emit-autoloads as nil (I do this). It is almost never necessary for general.el to create autoloads since most packages already correctly create autoloads. If a package doesn't create an autoload for a command, you can just do it manually and open an issue for the package to fix or add autoload cookies.

If it's not immediately obvious how to defer a package, you can always look at how doom does it. I plan on adding more equivalents for doom helpers related to autoloading packages to general at some point. Some functionality already exists in general.el (like run-once hook functions).

The more general startup time optimizations may also be worth taking a look at. At some point, you'll only be improving startup time by a small amount, but some of the optimizations are very simple to implement (e.g. disabling garbage collection during init, moving some configuration to the early init file, etc.). For example, see here and here. Just moving where I disabled the tool and menu bar saved me 0.2s. Even though I mostly use the daemon, I prefer to keep my emacs-init-time around 0.6s.

I disabled straight's bootstrapping check

A file-exists-p check takes almost no time (0.0002 seconds at most on my machine according to benchmark-run, usually much less), so it should be fine to leave this.

@blaenk
Copy link
Author

blaenk commented Apr 4, 2022

Wow thanks for all that information! I'll certainly give it a shot next time I get a chance, of course don't let that get in the way of closing this issue whenever.

Thanks again!

@noctuid
Copy link
Owner

noctuid commented Apr 9, 2022

If you get a chance, can you look at the new documentation I've added in the open PR and let me know if it is confusing or if I need to add more information?

@blaenk
Copy link
Author

blaenk commented Apr 10, 2022

Will do!

I also tried out some of the changes you mentioned such as adding early-init and disabling gc during init and it definitely does seem to have helped further! Thank you so much!

AmaiKinono added a commit to AmaiKinono/Tokimacs that referenced this issue Apr 10, 2022
…in which-key

For some reason when binding keys for a specific keymap using general, the
:which-key keyword doesn't work (My guess is it has something to do with
noctuid/general.el#502).

(<description> . <command>) is a supported definition (see the docstring of
define-key) and it works.
@noctuid
Copy link
Owner

noctuid commented Apr 10, 2022

See here also. Straight has an autoload concatenation feature builtin, but I didn't find it made a significant difference for me.

progfolio added a commit to progfolio/.emacs.d that referenced this issue Jun 1, 2022
Finally fixed thanks to the information in this issue:

noctuid/general.el#502

Binding our prefix keys to a keymap leads to a significant improvement in start up time.
This also eliminates the slowdown when loading evil after general.
progfolio added a commit to progfolio/.emacs.d that referenced this issue Jun 1, 2022
Finally fixed thanks to the information in this issue:

noctuid/general.el#502

Binding our prefix keys to a keymap leads to a significant improvement in start up time.
This also eliminates the slowdown when loading evil after general.
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

Successfully merging a pull request may close this issue.

2 participants