automatically factor anonymous functions with pred, 20% more effient cache strategy, more efficient hot path, added ability with must-cache-paths to error when a path can't be cached and get detailed information why

src/clj/com/rpl/specter.cljx

@@ -26,6 +26,8 @@
 (defn comp-paths [& paths]
   (i/comp-paths* (vec paths)))
 
+(def must-cache-paths! i/must-cache-paths!)
+
 ;; Selection functions
 
 (def ^{:doc "Version of select that takes in a path pre-compiled with comp-paths"}
@@ -403,15 +405,12 @@
   (transform* [aset structure next-fn]
     (i/filter-transform aset structure next-fn)))
 
-(defpath
+(def
   ^{:doc "Keeps the element only if it matches the supplied predicate. This is the
           late-bound parameterized version of using a function directly in a path."}
   pred
-  [afn]
+  i/pred*
-  (select* [this structure next-fn]
+  )
-    (i/filter-select afn structure next-fn))
-  (transform* [this structure next-fn]
-    (i/filter-transform afn structure next-fn)))
 
 (defpath
   ^{:doc "Navigates to the provided val if the structure is nil. Otherwise it stays

src/clj/com/rpl/specter/impl.cljx

@@ -633,26 +633,42 @@
    params-maker ; can be null
    ])
 
-(def CACHE
+(defonce PATH-CACHE
-  #+clj (java.util.concurrent.ConcurrentHashMap.)
+  #+clj (mutable-cell (java.util.HashMap.))
   #+cljs (atom {})
   )
 
-#+clj
+(def ^:dynamic *must-cache-paths* false)
-(defn add-cache! [k v]
+
-  (.put ^java.util.concurrent.ConcurrentHashMap CACHE k v))
+(defn must-cache-paths!
+  ([] (must-cache-paths! true))
+  ([v] (alter-var-root #'*must-cache-paths* (constantly v))))
 
 #+clj
-(defn get-cache [k]
+(defn add-path-cache! [k v]
-  (.get ^java.util.concurrent.ConcurrentHashMap CACHE k))
+  ;; This looks very inefficient but is actually the best approach
+  ;; without real inline caches. The total number of copies will be the
+  ;; number of Specter callsites (plus perhaps a few more if 
+  ;; multiple callsites attempt to be cached at exactly the same time).
+  ;; Eventually it will stabilize and there won't be any more garbage generated.
+  ;; The `select` performance using this cache strategy is ~20% faster for
+  ;; the [:a :b :c] path use case than ConcurrentHashMap. 
+  (let [newmap (java.util.HashMap. ^java.util.HashMap (get-cell PATH-CACHE))]
+    (.put newmap k v)
+    (set-cell! PATH-CACHE newmap)
+    ))
+
+#+clj
+(defn get-path-cache [k]
+  (.get ^java.util.HashMap (get-cell PATH-CACHE) k))
 
 #+cljs
 (defn add-cache! [k v]
-  (swap! CACHE (fn [m] (assoc m k v))))
+  (swap! PATH-CACHE (fn [m] (assoc m k v))))
 
 #+cljs
 (defn get-cache [k]
-  (get @CACHE k))
+  (get @PATH-CACHE k))
 
 (defn- extract-original-code [p]
   (cond
@@ -672,70 +688,109 @@
   (reset! failed-atom true)
   nil)
 
-(defn- magic-precompilation* [p params-atom failed-atom]
+(def pred*
-  (cond
+  (->ParamsNeededPath
-    (vector? p)
+    (->TransformFunctions
-    (mapv
+      RichPathExecutor
-      #(magic-precompilation* % params-atom failed-atom)
+      (fn [params params-idx vals structure next-fn]
-      p)
+        (let [afn (aget ^objects params params-idx)]
-
+          (if (afn structure)
-    (instance? LocalSym p)
+            (next-fn params (inc params-idx) vals structure)
-    (magic-fail! failed-atom)
+            )))
-
+      (fn [params params-idx vals structure next-fn]
-    (instance? VarUse p)
+        (let [afn (aget ^objects params params-idx)]
-    (let [v (:var p)
+          (if (afn structure)
-          vv (var-get v)]
+            (next-fn params (inc params-idx) vals structure)
-      (if (and (-> v meta :dynamic not)
+            structure
-               (valid-navigator? vv))
+            ))))
-        vv
+    1
-        (magic-fail! failed-atom)
-        ))
-
-    (instance? SpecialFormUse p)
-    (magic-fail! failed-atom)
-
-    (instance? FnInvocation p)
-    (let [op (:op p)
-          ps (:params p)]
-      (if (instance? VarUse op)
-        (let [v (:var op)
-              vv (var-get v)]
-          (if (-> v meta :dynamic)
-            (magic-fail! failed-atom)
-            (cond
-              (instance? ParamsNeededPath vv)
-              ;;TODO: if all params are constants, then just bind the path right here
-              ;;otherwise, add the params
-              (do
-                (swap! params-atom concat ps)
-                vv
-                )
-
-              (and (fn? vv) (-> vv meta :pathedfn))
-              (let [subpath (mapv #(magic-precompilation* % params-atom failed-atom)
-                                  ps)]
-                (if @failed-atom
-                  nil
-                  (apply vv subpath)
-                  ))
-
-              :else
-              (magic-fail! failed-atom)
-              )))
-        (magic-fail! failed-atom)
-        ))
-
-    :else
-    (magic-fail! failed-atom)
     ))
 
+(defn- magic-precompilation* [p params-atom failed-atom]
+  (let [magic-fail! (fn [& reason]
+                      (if *must-cache-paths*
+                        (println "Failed to cache path:" (apply str reason)))
+                      (reset! failed-atom true)
+                      nil)]
+    (cond
+      (vector? p)
+      (mapv
+        #(magic-precompilation* % params-atom failed-atom)
+        p)
+
+      (instance? LocalSym p)
+      (magic-fail! "Local symbol " (:sym p) " where navigator expected")
+
+      (instance? VarUse p)
+      (let [v (:var p)
+            vv (var-get v)]
+        (cond (-> v meta :dynamic)
+              (magic-fail! "Var " (:sym p) " is dynamic")
+              (valid-navigator? vv) vv
+              :else (magic-fail! "Var " (:sym p) " is not a navigator")
+              ))
+
+      (instance? SpecialFormUse p)
+      (if (-> p :code first (= 'fn*))
+        (do
+          (swap! params-atom conj (:code p))
+          pred*
+          )
+        (magic-fail! "Special form " (:code p) " where navigator expected")
+        )
+
+      (instance? FnInvocation p)
+      (let [op (:op p)
+            ps (:params p)]
+        (if (instance? VarUse op)
+          (let [v (:var op)
+                vv (var-get v)]
+            (if (-> v meta :dynamic)
+              (magic-fail! "Var " (:sym op) " is dynamic")
+              (cond
+                (instance? ParamsNeededPath vv)
+                ;;TODO: if all params are constants, then just bind the path right here
+                ;;otherwise, add the params
+                ;;  - could extend this to see if it contains nested function calls which
+                ;;    are only on constants
+                (do
+                  (swap! params-atom concat ps)
+                  vv
+                  )
+
+                (and (fn? vv) (-> vv meta :pathedfn))
+                (let [subpath (mapv #(magic-precompilation* % params-atom failed-atom)
+                                    ps)]
+                  (if @failed-atom
+                    nil
+                    (apply vv subpath)
+                    ))
+
+                :else
+                (magic-fail! "Var " (:sym op) " must be either a parameterized "
+                  "navigator or a higher order pathed constructor function")
+                )))
+          (magic-fail! "Code at " (extract-original-code p) " is in "
+            "function invocation position and must be either a parameterized "
+            "navigator or a higher order pathed constructor function"
+            )
+          ))
+
+      :else
+      (if (valid-navigator? p)
+        p
+        (magic-fail! "Constant " p " is not a valid navigator"))
+      )))
+
 (defn magic-precompilation [prepared-path used-locals]
   (let [params-atom (atom [])
         failed-atom (atom false)
         path (magic-precompilation* prepared-path params-atom failed-atom)
         ]
     (if @failed-atom
-      (->CachedPathInfo nil nil)
+      (if *must-cache-paths*
+        (throw-illegal "Failed to cache path")
+        (->CachedPathInfo nil nil))
       (let [precompiled (comp-paths* path)
             params-code (mapv extract-original-code @params-atom)
             array-sym (gensym "array")

src/clj/com/rpl/specter/macros.clj

@@ -315,8 +315,8 @@
   `(i/extend-protocolpath* ~protpath ~(protpath-sym protpath) ~(vec extensions)))
 
 (defmacro defpathedfn [name & args]
-  (let [[n args] (m/name-with-attributes name args)]
+  (let [[name args] (m/name-with-attributes name args)]
-    `(def ~n (vary-meta (fn ~@args) assoc :pathedfn true))))
+    `(def ~name (vary-meta (fn ~@args) assoc :pathedfn true))))
   
 
 (defn ic-prepare-path [locals-set path]
@@ -344,16 +344,23 @@
     path
     ))
 
-;; still possible to mess this up with alter-var-root!
+;; still possible to mess this up with alter-var-root
 (defmacro ic! [& path] ; "inline cache"
   (let [local-syms (-> &env keys set)
         used-locals (vec (i/walk-select local-syms vector path))
         prepared-path (ic-prepare-path local-syms (walk/macroexpand-all (vec path)))
-        ;; TODO: will turning this into a keyword make it faster?
+        ;; TODO: unclear if using long here versus string makes
+        ;; a significant difference
+        ;; - but using random longs creates possibility of collisions
+        ;; (birthday problem)
+        ;; - ideally could have a real inline cache that wouldn't
+        ;; have to do any hashing/equality checking at all
+        ;; - with invokedynamic here, could go directly to the code
+        ;; to invoke and/or parameterize the precompiled path without
+        ;; a bunch of checks beforehand
         cache-id (str (java.util.UUID/randomUUID))
         ]
-
+    `(let [info# (i/get-path-cache ~cache-id)
-    `(let [info# (i/get-cache ~cache-id)
            
            ^com.rpl.specter.impl.CachedPathInfo info#
             (if info#
@@ -362,19 +369,17 @@
                            ~prepared-path
                            ~(mapv (fn [e] `(quote ~e)) used-locals)
                            )]
-                (i/add-cache! ~cache-id info#)
+                (i/add-path-cache! ~cache-id info#)
                 info#
                 ))
 
            precompiled# (.-precompiled info#)
            params-maker# (.-params-maker info#)]
-       (cond (nil? precompiled#)
+       (if (some? precompiled#)
-             ~path
+         (if (nil? params-maker#)
-
+           precompiled#
-             (and precompiled# (nil? params-maker#))
+           (i/bind-params* precompiled# (params-maker# ~@used-locals) 0)
-             precompiled#
+           )
-
+         (i/comp-paths* ~(vec path))
-             :else
-             (i/bind-params* precompiled# (params-maker# ~@used-locals) 0)
          ))
   ))