bug#14769: [PATCH] optimize `concat's literals

["Mattias Engdegård" <mattiase@acm.org>]

[-- Attachment #1: Type: text/plain, Size: 866 bytes --]

19 juni 2019 kl. 03.43 skrev Noam Postavsky <npostavs@gmail.com>:
> 
> Mattias Engdegård <mattiase@acm.org> writes:
> 
>> +    (dolist (arg (cdr form))
> 
>> +                   (let ((val (eval arg)))
> 
>> +                          ;; Constant arg: concat with previous.
>> +                          (setq accum (concat accum val)))))
> 
> Hmm, I think the OP's patch is careful not to concat in a loop like
> this: it's O(n^2).  I guess for most human written code n is small
> enough that it doesn't matter, but I could imagine this slowing
> compilation of a very big rx macro.

Yes, I thought it wouldn't matter but you are right. Updated patch attached.

For that matter, ry performs this optimisation itself, but it's also nice to have it in the compiler, since concat is frequently used to split long string constants in elisp source.

[-- Attachment #2: 0001-Merge-consecutive-constant-concat-args-bug-14769.patch --]
[-- Type: application/octet-stream, Size: 2408 bytes --]

From d4c3bbce9472618b5e762a18af5ba1a2b1b79698 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mattias=20Engdeg=C3=A5rd?= <mattiase@acm.org>
Date: Sun, 16 Jun 2019 13:13:47 +0200
Subject: [PATCH] Merge consecutive constant `concat' args (bug#14769)

Suggested by Shigeru Fukaya <shigeru.fukaya@gmail.com>

* lisp/emacs-lisp/byte-opt.el (byte-optimize-concat): New.
(concat): Add byte-optimizer.
---
 lisp/emacs-lisp/byte-opt.el | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/lisp/emacs-lisp/byte-opt.el b/lisp/emacs-lisp/byte-opt.el
index b0aa407c8b..2e09601639 100644
--- a/lisp/emacs-lisp/byte-opt.el
+++ b/lisp/emacs-lisp/byte-opt.el
@@ -850,6 +850,33 @@ byte-optimize-memq
                           ',list)))))
     (byte-optimize-predicate form)))
 
+(defun byte-optimize-concat (form)
+  "Merge adjacent constant arguments to `concat'."
+  (let ((args (cdr form))
+        (newargs nil))
+    (while args
+      (let ((strings nil)
+            val)
+        (while (and args (macroexp-const-p (car args))
+                    (progn
+                      (setq val (eval (car args)))
+                      (and (or (stringp val)
+                               (and (or (listp val) (vectorp val))
+                                    (not (memq nil
+                                               (mapcar #'characterp val))))))))
+          (push val strings)
+          (setq args (cdr args)))
+        (when strings
+          (let ((s (apply #'concat (nreverse strings))))
+            (when (not (zerop (length s)))
+              (push s newargs)))))
+      (when args
+        (push (car args) newargs)
+        (setq args (cdr args))))
+    (if (= (length newargs) (length (cdr form)))
+        form          ; No improvement.
+      (cons 'concat (nreverse newargs)))))
+
 (put 'identity 'byte-optimizer 'byte-optimize-identity)
 (put 'memq 'byte-optimizer 'byte-optimize-memq)
 
@@ -892,6 +919,8 @@ byte-optimize-memq
 (put 'car-safe 'byte-optimizer 'byte-optimize-predicate)
 (put 'cdr-safe 'byte-optimizer 'byte-optimize-predicate)
 
+(put 'concat 'byte-optimizer 'byte-optimize-concat)
+
 ;; I'm not convinced that this is necessary.  Doesn't the optimizer loop
 ;; take care of this? - Jamie
 ;; I think this may some times be necessary to reduce ie (quote 5) to 5,
-- 
2.20.1 (Apple Git-117)