Skip to content

Defer elpaca form as a lambda #413

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

Closed
wants to merge 1 commit into from

Conversation

Stebalien
Copy link
Contributor

By deferring the elapca body form as a lambda instead of as an s-expression, the byte-compiler can process it. It won't matter for users who don't byte-compile their init files, but it provides a noticeable speedup for those of us that do.

To actually use this with a byte-compiled init file the user will need to add a few eval-and-compile forms to their Elpaca install snippet, but I wanted to keep this change as non-invasive as possible.

Also note: this change won't allow the native compiler to optimize functions defined in deferred forms.

This lets the byte-compiler process it. It won't matter for users who
don't byte-compile their init files, but it provides a noticeable speedup
for those of us that do.
@progfolio
Copy link
Owner

progfolio commented Jan 28, 2025

Thanks for the suggestion.
This idea has come up a couple times before. See #173 as well as the fix/thunk branch (both of which would need to be refactored to merge with the current master branch).
I'm not opposed to it so long as we're confident it works well with interpreted init files and does not impose a significant startup time cost in that case.

@Stebalien
Copy link
Contributor Author

I've tested it with:

  • No compilation (interpreted).
  • Byte-compilation only.
  • Native compilation.

I also tried it with both lambda and defsubst (using gensym to generate a new function name each time). Unfortunately, defsubst did run into some issues when byte-compiled (but not native compiled, IIRC?).

I've also tested it with macro expansion and defining a lambda before the macro is defined seems to work fine both when byte-compiled and when not byte-compiled.

I also didn't notice any increases in startup times when not byte-compiled and noticed a significant decrease in startup time when byte-compiled.

@Stebalien
Copy link
Contributor Author

It looks like both that branch and that PR are trying to solve the lexical scoping problem but don't actually give the byte-compiler access to the function definition. I based my approach here on both with-eval-after-load and use-package (both of which use functions/lambdas).

@Stebalien
Copy link
Contributor Author

Another note to add to this: Elpaca's quoting means that, if the user byte-compiles their init file, every single package declared by use-package be required at startup due to how use-package handles byte-compilation (leading to slow start times).

Basically, use-package will load the packages at compile time if and only if byte-compiling. The transformation is as follows:

(use-package emacs)

Into:

(progn
  (eval-and-compile
    (eval-when-compile
      (with-demoted-errors "Cannot load emacs: %S"
        nil (unless (featurep 'emacs) (load "emacs" nil t)))))
  (defvar use-package--warning250
    #'(lambda (keyword err)
        (let ((msg (format "%s/%s: %s" 'emacs keyword (error-message-string err))))
          (display-warning 'use-package msg :error))))
  (condition-case-unless-debug err nil (error (funcall use-package--warning250 :catch err))))

If and only if byte-compile-current-file is non-nil when expanding the use-package macro. You can test by placing point after a use-package form and evaluating (let ((byte-compile-current-file t)) (pp-macroexpand-last-sexp nil)).

This is usually fine as the byte-compiler will immediately see and evaluate the eval-when-compile and eval-and-compile forms. Unfortunately, when using the elpaca-use-package integration, Elpaca will quote the expanded use-package macro preventing the byte-compiler from seeing them. That means that these forms will get evaluated at runtime, eagerly loading the package at runtime.

@aikrahguzar
Copy link
Contributor

I don't byte-compile my init file but I would like to see this, since I think it will enable better flymake warnings for my init file.

@progfolio
Copy link
Owner

It looks like both that branch and that PR are trying to solve the lexical
scoping problem but don't actually give the byte-compiler access to the function
definition. I based my approach here on both with-eval-after-load and
use-package (both of which use functions/lambdas).

Here's a modified test case for the regression reported in #161 that was introduced by using thunks for the deferred forms. Note you'll need to be on latest version of Elpaca to run this test form, as I added a few keywords to make it easier to test forks of the project.

(elpaca-test
  :interactive t
  :early-init
  :repo "https://github.com/Stebalien/elpaca.git"
  :ref "steb/defer-lambda"
  :init
  (elpaca elpaca-use-package
    (elpaca-use-package-mode)
    (setq elpaca-use-package-by-default t))

  (use-package flycheck
    :defer t
    :init
    :hook ((prog-mode markdown-mode git-commit-mode text-mode) . flycheck-mode))

  (use-package quick-peek :config (message "`quick-peek' loaded"))

  (use-package flycheck-inline
    :hook (flycheck-mode . flycheck-inline-mode)
    :after quick-peek
    :config
    (setq flycheck-inline-display-function
          (lambda (msg pos err)
            (require 'quick-peek)
            (let* ((ov (quick-peek-overlay-ensure-at pos))
                   (contents (quick-peek-overlay-contents ov)))
              (setf (quick-peek-overlay-contents ov)
                    (concat contents (when contents "\n") msg))
              (quick-peek-update ov)))
          flycheck-inline-clear-function #'quick-peek-hide))

  (add-hook 'elpaca-after-init-hook
            (lambda ()
              (with-current-buffer (get-buffer-create "*scratch*")
                (erase-buffer)
                (insert "(defun err)")
                (goto-char (point-min))
                (display-buffer-same-window (current-buffer) nil)
                (flycheck-mode)))))

On my end I still see the error described in #161.
How would the user work around that error in their configuration?

Elpaca will quote the expanded use-package macro preventing the
byte-compiler from seeing them. That means that these forms will get evaluated
at runtime, eagerly loading the package at runtime.

I'm sure this could be worked around in elpaca-use-package if the other issues can be figured out. My main concern is that I don't want to require the majority of users, who do not byte-compile their init files, to jump through configuration hoops to prevent errors.

@Stebalien
Copy link
Contributor Author

Hm. I guess the difference is that, without Elpaca, quick-peek will get autoloaded at that time (I assume?).

The best fix I can think of is to have two modes:

  1. When not byte-compiling, quote/eval (preserve the current behavior).
  2. When byte-compiling (byte-compile-current-file is non-nil), synchronously assert that the order is already available and activate the package immediately before enqueuing the order.

Byte-compilation will fail if the package isn't available but, IMO, that's fine. The tricky part will be doing this without adding a bunch of complexity.

@Stebalien
Copy link
Contributor Author

If you're OK with that direction, I'm happy to submit a patch (although it may be a while before I get to it).

On the other hand, I don't want to add too much complexity to Elpaca just to solve this rather niche issue. I can just as easily replicate this patch in my own config with the following advice:

(eval-and-compile
  (define-advice elpaca (:override (order &rest body) lambda-body)
    (when body (setq body `(list (list 'funcall (lambda () ,@body)))))
    `(elpaca--expand-declaration ',order ,body)))

@progfolio
Copy link
Owner

progfolio commented Feb 8, 2025

If you're OK with that direction, I'm happy to submit a patch (although it may be a while before I get to it).

On the other hand, I don't want to add too much complexity to Elpaca just to solve this rather niche issue. I can just as easily replicate this patch in my own config with the following advice:

(eval-and-compile
  (define-advice elpaca (:override (order &rest body) lambda-body)
    (when body (setq body `(list (list 'funcall (lambda () ,@body)))))
    `(elpaca--expand-declaration ',order ,body)))

That's fine, there's no rush. As you say, it's a niche use case. If the patch is looking too complex if you do get around to it, a separate package could be authored by interested parties. It would basically be a separate global minor mode which enables the advice you've written.

@Stebalien
Copy link
Contributor Author

Closing for now. I'll open a new PR if I ever get around to fixing this, but it's not really a priority.

@Stebalien Stebalien closed this Mar 8, 2025
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 this pull request may close these issues.

3 participants