General causing after-load-alist to grow

[jgkamat]

This is a rather odd issue, and I'm a lot more confused than I usually am with issues I submit. This could be an issue on my end, but I'd like to at least put it out here in case others are unwittingly running into the same problem I am.

I was troubleshooting memory leaks in my emacs, and I found that after-load-alist was rather large, much larger than I would have expected. After looking at it's value, I discovered that most of the entries were related to eshell (even though I couldn't fully understand what's going on).

I then bisected my eshell config until I got a small reproducible init file.

;; Launch with 
;; emacs-dev -Q -l general-memleak.el
(package-initialize)
(require 'memory-usage)
(require 'general)

(add-hook
 'eshell-mode-hook
 #'jay/eshell-init)
(defun jay/eshell-init ()
  (general-define-key
   :keymaps '(eshell-mode-map)
   :states '(normal)
   "M-j" #'eshell-next-prompt
   "M-k" #'eshell-previous-prompt
   "M-a" #'tracking-next-buffer
   "M-e" #'eshell-smart-goto-end
   "M-RET" #'eshell-smart-goto-end
   "M-p" #'eshell-previous-input
   "M-n" #'eshell-next-input
   "M-r" #'counsel-esh-history
   "M-t" #'counsel-esh-directory
   "<tab>" #'completion-at-point))

;; Run me: (dotimes (x 100) (eshell) (kill-buffer))

When launching with this init file, first run M-x memory-usage, which will show you some basic stats. Then M-x memory-usage-find-large-variables, and note how after-load-alist is not in the list (it is too small to be listed).

After running the dotimes script, run memory-usage-find-variables again, and compare it to the previous run. For me, after-load-alist had grown to the largest variable in the list by almost an order of magnitude.

I don't know much about the general internals, but I was able to find out that:

• Using evil-define-key directly dosen't leak
• Removing :states prevents the leak
• Removing entries does not prevent the leak, but makes it less pronounced. Adding more unique entries seems to make the leak worse.
• Not tying things to eshell-mode-hook (ie: running them directly) does not leak
  I'd like to provide more information, but I don't know enough about general (and it's quite complex at this point). Please let me know if I can help in any way.

My best guess is that this is related to some eval-after-load (or other deferred loading) stuff, but I'm not sure. I found this in the docs:

Note that STATES and KEYMAPS can either be lists or single symbols. If any keymap does not exist, those keybindings will be deferred until the keymap does exist, so using `eval-after-load' is not necessary with this function.

How is this done? As far as I know eshell-mode-map is available at that point, and (boundp 'eshell-mode-map) seems to be true.

[noctuid]

I can replicate this, but I don't have any ideas initially.

How is this done?

It's done in the same way as evil-define-key (by adding to after-load-functions when the keymap isn't bound; see evil-delay). I tested your example, and this isn't happening (the keybindings are made immediately), so this probably isn't the issue.

Removing :states prevents the leak

The only significant difference that comes to mind is that when :states is used, (with-eval-after-load 'evil ... is called once for every key/def pair, but this shouldn't be an issue when evil is loaded (and I can replicate the issue when evil is loaded).

[jgkamat]

The only significant difference that comes to mind is that when :states is used, (with-eval-after-load 'evil ... is called once for every key/def pair, but this shouldn't be an issue when evil is loaded (and I can replicate the issue when evil is loaded).

I think that was the hint I needed, I did a bit of bisecting in general.el, and it looks like this line is "causing" the issue.

Weirdly, sticking a require or removing the body dosen't help, so

(require 'evil)
(evil-define-key* state keymap key def)
(with-eval-after-load 'evil
)

leaks, but

(require 'evil)
(evil-define-key* state keymap key def)
;; (with-eval-after-load 'evil
;;   )

is fine? I'm more confused than I was before at this point... I'll keep investigating this some more, I'm guessing it has to do something with lexical scope and closures. From a basic test expanding the with-eval-after-load with an empty lambda causes the issue, but inserting an empty function defined in the top level is fine...

[jgkamat]

It turns out that this can reproduce it too:

;;; memleak.el --- Test -*- lexical-binding: t -*-

(package-initialize)
(require 'memory-usage)
(require 'subr-x)

(defun memleak ()
  (dotimes (x 1000)
    (eval-after-load 'subr-x (lambda ()))))
(memleak)

(memory-usage-find-large-variables)

so I don't think this is an issue in general at all. Feel free to close this if you wish, I'll probably file a bug report about it once I get more information...

(EDIT: this actually seems intended, which is weird). It might be because "If a matching file is loaded again, FORM will be evaluated again." I think this might not be the behavior general wants :/

[noctuid]

I think this might not be the behavior general wants :/

I've had a users who didn't want general to require evil but instead wait until it was loaded. It seems to me like eval-after-load should provide an option to prevent it from adding to after-load-alist when specified file has already been loaded; that seems like the behavior most people would want. I can create an alternate macro as a workaround that runs the code directly if the feature already exists and use that instead if you'd like.

[jgkamat]

I can create an alternate macro as a workaround that runs the code directly if the feature already exists and use that instead if you'd like.

That would be awesome, if you don't think it would break anything! I can also work on this eventually, but I probably won't have free time for the next couple of weeks.

I agree with you on the option to eval-after-load as well. I'll post to the ML at some point asking for the option.

[noctuid]

I don't think this should cause any problems. Since with-eval-after-load is just being used to prevent the need to load optional dependencies, I don't see any reason to add to after-load-alist when the optional dependencies are already loaded. I went ahead and made the change.

This won't fix the problem if evil hasn't been loaded when eshell is run, but for that case, I'd say the fault is more with eshell for its buggy keymap handling making it necessary to run setup for every new instance. Otherwise duplicates wouldn't be repeatedly added to after-load-alist.