Subject: Byte compilation of Emacs autoloads
From: Maxim Cournoyer <[email-masked]>
To: guix-patches <[email-masked]>
Date: Sun, 15 Dec 2019 13:30:36 +0900
List-Id: <debbugs-submit.debbugs.gnu.org>

Hello,

The following adds support for byte compiling the generated autoload
files as well as the site-lisp directory of Emacs (the one containing
the site-start.el and guix-emacs.el files).

The performance gain is roughly 30%, as expected for byte compiled
Elisp.  The following command was used for benchmarking:

--8<---------------cut here---------------start------------->8---
./pre-inst-env guix environment -v 10 -m ~/100-emacs-packages-manifest.scm -- emacs -Q --batch --eval "(progn (require 'guix-emacs) (message \"%s\" (benchmark-run-compiled 10 (guix-emacs-autoload-packages))))"
--8<---------------cut here---------------end--------------->8---

It loads the autoloads of ~100 Emacs packages 10 times (~1000 autoloads
files).  The manifest file 100-emacs-packages-manifest.scm is attached.

The above command for Guix before the changes (commit e19a539):

--8<---------------cut here---------------start------------->8---
[...]
Loading /gnu/store/lj09yx56hgsq9l4ajk5wlxsk4vbrympk-profile/share/emacs/site-lisp/undercover-autoloads.el (source)...
Loading /gnu/store/lj09yx56hgsq9l4ajk5wlxsk4vbrympk-profile/share/emacs/site-lisp/with-editor-autoloads.el (source)...
Loading /gnu/store/lj09yx56hgsq9l4ajk5wlxsk4vbrympk-profile/share/emacs/site-lisp/ws-butler-autoloads.el (source)...
Loading /gnu/store/lj09yx56hgsq9l4ajk5wlxsk4vbrympk-profile/share/emacs/site-lisp/yasnippet-autoloads.el (source)...
(9.971024566 159 2.763721703999999)
--8<---------------cut here---------------end--------------->8---

After the changes:

--8<---------------cut here---------------start------------->8---
Loading /gnu/store/fwji52vg31xmdkc2z5cbjrfza7fxndr5-profile/share/emacs/site-lisp/undercover-autoloads...
Loading /gnu/store/fwji52vg31xmdkc2z5cbjrfza7fxndr5-profile/share/emacs/site-lisp/with-editor-autoloads...
Loading /gnu/store/fwji52vg31xmdkc2z5cbjrfza7fxndr5-profile/share/emacs/site-lisp/ws-butler-autoloads...
Loading /gnu/store/fwji52vg31xmdkc2z5cbjrfza7fxndr5-profile/share/emacs/site-lisp/yasnippet-autoloads...
(7.435941036 143 2.406266451999999)
--8<---------------cut here---------------end--------------->8---

I think it's neat that all of our Elisp is byte compiled; it's something
that can be done without fear on Guix, given that when the Emacs package itself
changes, all the Emacs packages are rebuilt.  It could also find issues we'd not
know existed otherwise, as I found out for the emacs-cl-generic package.

From a54e9beb22d2d5102e48e26834b19bb2603f50b5 Mon Sep 17 00:00:00 2001
From: Maxim Cournoyer <[email-masked]>
Date: Thu, 5 Dec 2019 13:40:32 +0900
Subject: [PATCH 1/3] gnu: emacs: Byte compile the site-lisp directory.

* gnu/packages/emacs.scm (emacs): Use the (gnu build emacs-utils) module.
Byte compile the site-lisp directory as part of the 'install-site-start'
phase.
---
 gnu/packages/emacs.scm | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/gnu/packages/emacs.scm b/gnu/packages/emacs.scm
index eba7f88551..fcf966839b 100644
--- a/gnu/packages/emacs.scm
+++ b/gnu/packages/emacs.scm
@@ -119,6 +119,10 @@
      `(#:tests? #f                      ; no check target
        #:configure-flags (list "--with-modules"
                                "--disable-build-details")
+       #:modules ((guix build emacs-utils)
+                  ,@%glib-or-gtk-build-system-modules)
+       #:imported-modules ((guix build emacs-utils)
+                           ,@%glib-or-gtk-build-system-modules)
        #:phases
        (modify-phases %standard-phases
          (add-before 'configure 'fix-/bin/pwd
@@ -135,7 +139,8 @@
            ;; automatically found.
            (lambda* (#:key inputs outputs #:allow-other-keys)
              (let* ((out      (assoc-ref outputs "out"))
-                    (lisp-dir (string-append out "/share/emacs/site-lisp")))
+                    (lisp-dir (string-append out "/share/emacs/site-lisp"))
+                    (emacs (string-append out "/bin/emacs")))
                (copy-file (assoc-ref inputs "guix-emacs.el")
                           (string-append lisp-dir "/guix-emacs.el"))
                (with-output-to-file (string-append lisp-dir "/site-start.el")
@@ -148,6 +153,9 @@
                ;; share/emacs/site-lisp union when added to EMACSLOADPATH,
                ;; which leads to conflicts.
                (delete-file (string-append lisp-dir "/subdirs.el"))
+               ;; Byte compile the site-start files.
+               (parameterize ((%emacs emacs))
+                 (emacs-byte-compile-directory lisp-dir))
                #t))))))
     (inputs
      `(("gnutls" ,gnutls)
-- 
2.23.0

From 8fa11e7d52798494e0ea4d19ae8898bf949a18b8 Mon Sep 17 00:00:00 2001
From: Maxim Cournoyer <[email-masked]>
Date: Fri, 6 Dec 2019 15:12:32 +0900
Subject: [PATCH 2/3] emacs-build-system: Byte compile the autoload files.

* guix/build/emacs-build-system.scm (enable-autoloads-compilation)
(validate-compiled-autoloads): Add procedures.
(%standard-phases): Register the new procedures.
* gnu/packages/aux-files/emacs/guix-emacs.el (guix-emacs-find-autoloads):
Remove duplicates in the list of autoload files found.
* guix/build/emacs-utils.scm (expr->string): Add procedure.
(emacs-batch-eval, emacs-batch-edit-file): Use it.
---
 gnu/packages/aux-files/emacs/guix-emacs.el |  5 +++--
 guix/build/emacs-build-system.scm          | 22 ++++++++++++++++++++--
 guix/build/emacs-utils.scm                 | 10 ++++++++--
 3 files changed, 31 insertions(+), 6 deletions(-)

diff --git a/gnu/packages/aux-files/emacs/guix-emacs.el b/gnu/packages/aux-files/emacs/guix-emacs.el
index 05fc9709b6..25a87ee52d 100644
--- a/gnu/packages/aux-files/emacs/guix-emacs.el
+++ b/gnu/packages/aux-files/emacs/guix-emacs.el
@@ -35,8 +35,9 @@
   "Return a list of Emacs 'autoloads' files in DIRECTORY.
 The files in the list do not have extensions (.el, .elc)."
   ;; `directory-files' doesn't honor group in regexp.
-  (mapcar #'file-name-sans-extension
-          (directory-files directory 'full-name guix-emacs-autoloads-regexp)))
+  (delete-dups (mapcar #'file-name-sans-extension
+                       (directory-files directory 'full-name
+                                        guix-emacs-autoloads-regexp))))
 
 ;;;###autoload
 (defun guix-emacs-autoload-packages ()
diff --git a/guix/build/emacs-build-system.scm b/guix/build/emacs-build-system.scm
index 09de244993..219310cf08 100644
--- a/guix/build/emacs-build-system.scm
+++ b/guix/build/emacs-build-system.scm
@@ -225,6 +225,21 @@ parallel. PARALLEL-TESTS? is ignored when using a non-make TEST-COMMAND."
     (parameterize ((%emacs emacs))
       (emacs-generate-autoloads elpa-name site-lisp))))
 
+(define* (enable-autoloads-compilation #:key outputs #:allow-other-keys)
+  "Remove the NO-BYTE-COMPILATION local variable embedded in the generated
+autoload files."
+  (let* ((out (assoc-ref outputs "out"))
+         (autoloads (find-files out "-autoloads.el$")))
+    (substitute* autoloads
+      ((";; no-byte-compile.*") ""))
+    #t))
+
+(define* (validate-compiled-autoloads #:key outputs #:allow-other-keys)
+  "Verify whether the byte compiled autoloads load fine."
+  (let* ((out (assoc-ref outputs "out"))
+         (autoloads (find-files out "-autoloads.elc$")))
+    (emacs-batch-eval (format #f "(mapc #'load '~s)" autoloads))))
+
 (define (emacs-package? name)
   "Check if NAME correspond to the name of an Emacs package."
   (string-prefix? "emacs-" name))
@@ -253,10 +268,13 @@ second hyphen.  This corresponds to 'name-version' as used in ELPA packages."
     (replace 'check check)
     (replace 'install install)
     (add-after 'install 'make-autoloads make-autoloads)
-    (add-after 'make-autoloads 'patch-el-files patch-el-files)
+    (add-after 'make-autoloads 'enable-autoloads-compilation
+      enable-autoloads-compilation)
+    (add-after 'enable-autoloads-compilation 'patch-el-files patch-el-files)
     ;; The .el files are byte compiled directly in the store.
     (add-after 'patch-el-files 'build build)
-    (add-after 'build 'move-doc move-doc)))
+    (add-after 'build 'validate-compiled-autoloads validate-compiled-autoloads)
+    (add-after 'validate-compiled-autoloads 'move-doc move-doc)))
 
 (define* (emacs-build #:key inputs (phases %standard-phases)
                       #:allow-other-keys #:rest args)
diff --git a/guix/build/emacs-utils.scm b/guix/build/emacs-utils.scm
index fdacd30dd6..59467d632e 100644
--- a/guix/build/emacs-utils.scm
+++ b/guix/build/emacs-utils.scm
@@ -39,16 +39,22 @@
   ;; The `emacs' command.
   (make-parameter "emacs"))
 
+(define (expr->string expr)
+  "Converts EXPR, an expression, into a string."
+  (if (string? expr)
+      expr
+      (format #f "~s" expr)))
+
 (define (emacs-batch-eval expr)
   "Run Emacs in batch mode, and execute the elisp code EXPR."
   (invoke (%emacs) "--quick" "--batch"
-          (format #f "--eval=~S" expr)))
+          (string-append "--eval=" (expr->string expr))))
 
 (define (emacs-batch-edit-file file expr)
   "Load FILE in Emacs using batch mode, and execute the elisp code EXPR."
   (invoke (%emacs) "--quick" "--batch"
           (string-append "--visit=" file)
-          (format #f "--eval=~S" expr)))
+          (string-append "--eval=" (expr->string expr))))
 
 (define (emacs-generate-autoloads name directory)
   "Generate autoloads for Emacs package NAME placed in DIRECTORY."
-- 
2.23.0

From c3581a39268a24550fe6d0f2c3f1b630c2d6a5fd Mon Sep 17 00:00:00 2001
From: Maxim Cournoyer <[email-masked]>
Date: Sat, 7 Dec 2019 02:47:25 +0900
Subject: [PATCH 3/3] gnu: emacs-cl-generic: Disable byte compilation of its
 autoload file.

*gnu/packages/emacs-xyz.scm (emacs-cl-generic)[phases]
{enable-autoloads-compilation}: Delete.
---
 gnu/packages/emacs-xyz.scm | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/gnu/packages/emacs-xyz.scm b/gnu/packages/emacs-xyz.scm
index 2b8c143d4a..5fa78030b8 100644
--- a/gnu/packages/emacs-xyz.scm
+++ b/gnu/packages/emacs-xyz.scm
@@ -11862,6 +11862,10 @@ match and total match information in the mode-line in various search modes.")
         (base32
          "0vb338bhjpsnrf60qgxny4z5rjrnifahnrv9axd4shay89d894zq"))))
     (build-system emacs-build-system)
+    ;; Byte compilation of the autoload file fails.
+    (arguments
+     `(#:phases (modify-phases %standard-phases
+                  (delete 'enable-autoloads-compilation))))
     (home-page "https://elpa.gnu.org/packages/seq.html")
     (synopsis
      "Forward @code{cl-generic} compatibility for Emacs before version 25")
-- 
2.23.0

Maxim