(GitHub Action) Improve map and map-ref. (#209)

This commit was copied from the master branch.

Commit: 64a95bc
Author: okamsn <[email protected]>
Date: 2024-09-11 23:05:21 +0000

Improve `map` and `map-ref`. (#209)

- Check as we go instead of using `seq-uniq` immediately, which tests have shown
  to always faster for the tested 10, 100, and 1000 entries.

- Allow `unique` to be evaluated at run time.  Optimize when we know
  when it is `nil` or `non-nil` at compile time.

- Clarify Org docs.

See also issue #179. Closes #179.

CHANGELOG.md

@@ -42,14 +42,24 @@ This document describes the user-facing changes to Loopy.
 
 - Make `sequence-index` the default name and `seq-index` an alias ([#126, #206]).
 
+- Allow the `unique` keyword argument of the commands `map` and `map-ref` to be
+  evaluable at run time, instead of just checked at compile time ([#209]).
+
+### Improvements
+
+- The `map` and `map-ref` commands now check for duplicate keys step by step,
+  instead of all at once at the start of the loop ([#209], [#179]).  Testing
+  showed that this is consistently faster than the old method.
 
 [#126]: https://github.com/okamsn/loopy/issues/126
 [#168]: https://github.com/okamsn/loopy/issues/168
 [#169]: https://github.com/okamsn/loopy/issues/169
+[#179]: https://github.com/okamsn/loopy/issues/179
 [#203]: https://github.com/okamsn/loopy/pull/203
 [#205]: https://github.com/okamsn/loopy/pull/205
 [#206]: https://github.com/okamsn/loopy/pull/206
 [#207]: https://github.com/okamsn/loopy/pull/207
+[#209]: https://github.com/okamsn/loopy/pull/209
 
 
 ## 0.13.0

README.org

@@ -39,6 +39,8 @@ please let me know.
      list of built-in aliases in the future.  They can still be added to the
      list of known aliases using ~loopy-defalias~.  See the changelog for more
      information.
+   - The =:unique= keyword argument of the =map= and =map-ref= commands can now
+     be evaluable at run time, similar to most other keyword arguments.
  - Version 0.13.0:
    - The deprecated =:init= keyword argument has been removed.  Use the =with=
      special macro argument instead.

doc/loopy-doc.org

@@ -1978,9 +1978,10 @@ source sequences.
   order in which the key-value pairs are found.  There is no guarantee that they
   be in the same order each time.
 
-  These pairs are created before the loop begins.  In other words, the map
-  =EXPR= is not processed progressively, but all at once.  Therefore, this
-  command can have a noticeable start-up cost when working with very large maps.
+  These pairs are created before the loop begins via ~map-pairs~.  In other
+  words, the map =EXPR= is not processed progressively, but all at once.
+  Therefore, this command can have a noticeable start-up cost when working with
+  very large maps.
 
   #+begin_src emacs-lisp
     ;; => ((a . 1) (b . 2))
@@ -2426,8 +2427,8 @@ the accessed index during the loop.
   of other commands.  This is not the same as the =key= keyword parameter of the
   accumulation commands.
 
-  Like in the command =map=, the keys of the map are generated before the
-  loop is run, which can be expensive for large maps.
+  Like in the command =map=, the keys of the map are generated via the function
+  ~map-keys~ before the loop is run, which can be expensive for large maps.
 
   Similar to =map=, any duplicate keys are ignored by default.  This can be
   disabled by setting the =unique= keyword argument to nil, though note that

doc/loopy.texi

@@ -705,7 +705,7 @@ You should keep in mind that commands are evaluated in order.  This means that
 attempting something like the below example might not do what you expect, as @samp{i}
 is assigned a value from the list after collecting @samp{i} into @samp{coll}.
 
-@float Listing,org5119497
+@float Listing,orgfa9540f
 @lisp
 ;; => (nil 1 2)
 (loopy (collect coll i)
@@ -887,7 +887,7 @@ the flag @samp{dash} provided by the package @samp{loopy-dash}.
 
 Below are two examples of destructuring in @code{cl-loop} and @code{loopy}.
 
-@float Listing,orgf3a9f7c
+@float Listing,orgb706e77
 @lisp
 ;; => (1 2 3 4)
 (cl-loop for (i . j) in '((1 . 2) (3 . 4))
@@ -902,7 +902,7 @@ Below are two examples of destructuring in @code{cl-loop} and @code{loopy}.
 @caption{Destructuring values in a list.}
 @end float
 
-@float Listing,org1bf5dc0
+@float Listing,orgcc60e47
 @lisp
 ;; => (1 2 3 4)
 (cl-loop for elem in '((1 . 2) (3 . 4))
@@ -2150,9 +2150,10 @@ In general, as a map in not necessarily a sequence, you should not rely on the
 order in which the key-value pairs are found.  There is no guarantee that they
 be in the same order each time.
 
-These pairs are created before the loop begins.  In other words, the map
-@samp{EXPR} is not processed progressively, but all at once.  Therefore, this
-command can have a noticeable start-up cost when working with very large maps.
+These pairs are created before the loop begins via @code{map-pairs}.  In other
+words, the map @samp{EXPR} is not processed progressively, but all at once.
+Therefore, this command can have a noticeable start-up cost when working with
+very large maps.
 
 @lisp
 ;; => ((a . 1) (b . 2))
@@ -2629,8 +2630,8 @@ place referred to by @samp{VAR}.  This is similar to the @samp{index} keyword pa
 of other commands.  This is not the same as the @samp{key} keyword parameter of the
 accumulation commands.
 
-Like in the command @samp{map}, the keys of the map are generated before the
-loop is run, which can be expensive for large maps.
+Like in the command @samp{map}, the keys of the map are generated via the function
+@code{map-keys} before the loop is run, which can be expensive for large maps.
 
 Similar to @samp{map}, any duplicate keys are ignored by default.  This can be
 disabled by setting the @samp{unique} keyword argument to nil, though note that
@@ -4653,7 +4654,7 @@ using the @code{let*} special form.
 This method recognizes all commands and their aliases in the user option
 @code{loopy-aliases}.
 
-@float Listing,orgb546acb
+@float Listing,orgab882ee
 @lisp
 ;; => ((1 2 3) (-3 -2 -1) (0))
 (loopy-iter (arg accum-opt positives negatives other)

loopy-commands.el

@@ -897,50 +897,86 @@ BY is the function to use to move through the list (default `cdr')."
          (setq ,list-val (funcall ,list-func ,list-val)))))))
 
 ;;;;;; Map
-;; TODO: Instead of using `seq-uniq' at the start,
-;;       check as we go.
 (cl-defun loopy--parse-map-command ((name var val &key (unique t)))
-  "Parse the `map' loop command.
+  "Parse the `map' loop command as `(map VAR EXPR &key (unique t))'.
 
 Iterates through an alist of (key . value) dotted pairs,
 extracted from a hash-map, association list, property list, or
 vector using the library `map.el'."
   (when loopy--in-sub-level
     (loopy--signal-bad-iter name 'map))
-  (let ((value-holder (gensym "map-")))
-    `((loopy--iteration-vars
-       (,value-holder ,(if unique
-                           `(seq-uniq (map-pairs ,val) #'loopy--car-equal-car)
-                         `(map-pairs ,val))))
-      ,@(loopy--destructure-for-iteration-command var `(car ,value-holder))
-      ;; NOTE: The benchmarks show that `consp' is faster than no `consp',
+  (loopy--instr-let-var* ((value-holder `(map-pairs ,val)))
+      loopy--iteration-vars
+    `(;; NOTE: The benchmarks show that `consp' is faster than no `consp',
       ;;       at least for some commands.
       (loopy--pre-conditions (consp ,value-holder))
-      (loopy--latter-body (setq ,value-holder (cdr ,value-holder))))))
+      (loopy--latter-body (setq ,value-holder (cdr ,value-holder)))
+      ,@(pcase unique
+          ('nil
+           (loopy--destructure-for-iteration-command var `(car ,value-holder)))
+          ('t
+           (loopy--instr-let-var* ((key-list nil))
+               loopy--iteration-vars
+             (loopy--destructure-for-iteration-command
+              var `(progn
+                     (while (member (caar ,value-holder) ,key-list)
+                       (setq ,value-holder (cdr ,value-holder)))
+                     (push (caar ,value-holder) ,key-list)
+                     (car ,value-holder)))))
+          (_
+           (loopy--instr-let-var* ((key-list nil)
+                                   (test-fn `(if ,unique
+                                                 #'member
+                                               #'ignore)))
+               loopy--iteration-vars
+             (loopy--destructure-for-iteration-command
+              var `(progn
+                     (while (funcall ,test-fn (caar ,value-holder) ,key-list)
+                       (setq ,value-holder (cdr ,value-holder)))
+                     (push (caar ,value-holder) ,key-list)
+                     (car ,value-holder)))))))))
 
 ;;;;;; Map-Ref
 (cl-defun loopy--parse-map-ref-command ((name var val &key key (unique t)))
-  "Parse the `map-ref' command as (map-ref VAR VAL).
+  "Parse the `map-ref' command as (map-ref VAR VAL &key key (unique t)).
 
 KEY is a variable name in which to store the current key.
 
 Uses `map-elt' as a `setf'-able place, iterating through the
 map's keys.  Duplicate keys are ignored."
   (when loopy--in-sub-level
     (loopy--signal-bad-iter name 'map-ref))
-  (let ((key-list (gensym "map-ref-keys")))
-    `((loopy--iteration-vars (,key-list ,(if unique
-                                             `(seq-uniq (map-keys ,val))
-                                           `(map-keys ,val))))
+  (loopy--instr-let-var* ((key-list `(map-keys ,val)))
+      loopy--iteration-vars
+    `(;; NOTE: The benchmarks show that `consp' is faster than no `consp',
+      ;;       at least for some commands.
+      (loopy--pre-conditions (consp ,key-list))
+      (loopy--latter-body (setq ,key-list (cdr ,key-list)))
+      ,@(pcase unique
+          ;; We don't need to do anything if we don't care about uniqueness.
+          ('nil nil)
+          ;; If UNIQUE is not evaluable code and is not `nil', then we know that
+          ;; we can use `member' directly.
+          ('t
+           (loopy--instr-let-var* ((found-keys nil))
+               loopy--iteration-vars
+             `((loopy--main-body (while (member (car ,key-list) ,found-keys)
+                                   (setq ,key-list (cdr ,key-list))))
+               (loopy--main-body (push (car ,key-list) ,found-keys)))))
+          (_
+           (loopy--instr-let-var* ((found-keys nil)
+                                   (test-fn `(if ,unique
+                                                 #'member
+                                               #'ignore)))
+               loopy--iteration-vars
+             `((loopy--main-body (while (funcall ,test-fn (car ,key-list) ,found-keys)
+                                   (setq ,key-list (cdr ,key-list))))
+               (loopy--main-body (push (car ,key-list) ,found-keys))))))
       ,@(when key
           `((loopy--iteration-vars (,key nil))
             (loopy--main-body (setq ,key (car ,key-list)))))
       ,@(loopy--destructure-for-generalized-command
-         var `(map-elt ,val ,(or key `(car ,key-list))))
-      ;; NOTE: The benchmarks show that `consp' is faster than no `consp',
-      ;;       at least for some commands.
-      (loopy--pre-conditions (consp ,key-list))
-      (loopy--latter-body (setq ,key-list (cdr ,key-list))))))
+         var `(map-elt ,val ,(or key `(car ,key-list)))))))
 
 ;;;;;; Numbers
 

tests/tests.el

@@ -1717,7 +1717,7 @@ Using numbers directly will use less variables and more efficient code."
   :iter-bare ((_map . (mapping mapping-pairs))
               (collect . collecting)))
 
-(loopy-deftest map-:unique-t
+(loopy-deftest map-:unique-t-1
   :doc "`:unique' it `t' by default."
   :result '((a . 1) (b . 2) (c . 3))
   :multi-body t
@@ -1732,7 +1732,18 @@ Using numbers directly will use less variables and more efficient code."
   :iter-bare ((map-pairs . mapping-pairs)
               (collect . collecting)))
 
-(loopy-deftest map-:unique-nil
+(loopy-deftest map-:unique-t-2
+  :doc "Check that optimization to avoid `funcall' works."
+  :result t
+  :wrap ((x . `(not (string-match-p
+                     "funcall"
+                     (format "%S" (macroexpand-all (quote ,x) nil))))))
+  :body ((map-pairs i map :unique t))
+  :loopy t
+  :iter-keyword (map-pairs)
+  :iter-bare ((map-pairs . mapping-pairs)))
+
+(loopy-deftest map-:unique-nil-1
   :doc "`:unique' it `t' by default.  Test when `nil'."
   :result '((a . 1) (a . 27) (b . 2) (c . 3))
   :body ((map-pairs pair '((a . 1) (a . 27) (b . 2) (c . 3)) :unique nil)
@@ -1743,6 +1754,29 @@ Using numbers directly will use less variables and more efficient code."
   :iter-bare ((map-pairs . mapping-pairs)
               (collect . collecting)))
 
+(loopy-deftest map-:unique-nil-2
+  :doc "Check that optimization to avoid `funcall' works."
+  :result t
+  :wrap ((x . `(not (string-match-p
+                     "funcall"
+                     (format "%S" (macroexpand-all (quote ,x) nil))))))
+  :body ((map-pairs i map :unique nil))
+  :loopy t
+  :iter-keyword (map-pairs)
+  :iter-bare ((map-pairs . mapping-pairs)))
+
+(loopy-deftest map-:unique-var
+  :doc "`:unique' it `t' by default.  Test when `nil'."
+  :result '((a . 1) (a . 27) (b . 2) (c . 3))
+  :body ((with (cat nil))
+         (map-pairs pair '((a . 1) (a . 27) (b . 2) (c . 3)) :unique cat)
+         (collect coll pair)
+         (finally-return coll))
+  :loopy t
+  :iter-keyword (map-pairs collect)
+  :iter-bare ((map-pairs . mapping-pairs)
+              (collect . collecting)))
+
 (loopy-deftest map-destructuring
   :doc  "Check that `map' implements destructuring, not destructuring itself."
   :result '((a b) (1 2))
@@ -1782,7 +1816,7 @@ Using numbers directly will use less variables and more efficient code."
               (do . ignore)
               (collect . collecting)))
 
-(loopy-deftest map-ref-:unique-t
+(loopy-deftest map-ref-:unique-t-1
   :doc "`:unique' is `t' by default."
   :result '(:a 8 :a 2 :b 10)
   :multi-body t
@@ -1800,7 +1834,18 @@ Using numbers directly will use less variables and more efficient code."
               (do . ignore)
               (collect . collecting)))
 
-(loopy-deftest map-ref-:unique-nil
+(loopy-deftest map-ref-:unique-t-2
+  :doc "Check that optimization to avoid `funcall' works."
+  :result t
+  :wrap ((x . `(not (string-match-p
+                     "funcall"
+                     (format "%S" (macroexpand-all (quote ,x) nil))))))
+  :body ((map-ref i map :unique t))
+  :loopy t
+  :iter-keyword (map-ref)
+  :iter-bare ((map-ref . mapping-ref)))
+
+(loopy-deftest map-ref-:unique-nil-1
   :doc "Fist `:a' becomes 15 because it gets found twice by `setf'."
   :result '(:a 15 :a 2 :b 10)
   :body ((with (map (list :a 1 :a 2 :b 3)))
@@ -1813,6 +1858,31 @@ Using numbers directly will use less variables and more efficient code."
               (do . ignore)
               (collect . collecting)))
 
+(loopy-deftest map-ref-:unique-nil-2
+  :doc "Check that optimization to avoid `funcall' works."
+  :result t
+  :wrap ((x . `(not (string-match-p
+                     "funcall"
+                     (format "%S" (macroexpand-all (quote ,x) nil))))))
+  :body ((map-ref i map :unique nil))
+  :loopy t
+  :iter-keyword (map-ref)
+  :iter-bare ((map-ref . mapping-ref)))
+
+(loopy-deftest map-ref-:unique-var
+  :doc "Fist `:a' becomes 15 because it gets found twice by `setf'."
+  :result '(:a 15 :a 2 :b 10)
+  :body ((with (map (list :a 1 :a 2 :b 3))
+               (cat nil))
+         (map-ref i map :unique cat)
+         (do (cl-incf i 7))
+         (finally-return map))
+  :loopy t
+  :iter-keyword (map-ref do collect)
+  :iter-bare ((map-ref . mapping-ref)
+              (do . ignore)
+              (collect . collecting)))
+
 (loopy-deftest map-ref-destr
   :doc "Check that `map-ref' implements destructuring, not the destructuring itself."
   :result [[7 8] [7 8]]