From b3e581f7371a96b8cb2098ce2e7049349508af4e Mon Sep 17 00:00:00 2001 From: Alex Engelberg Date: Sun, 5 Jun 2016 00:46:21 -0700 Subject: [PATCH 01/21] WIP, transient navigators --- src/clj/com/rpl/specter/impl.cljx | 49 ++++++++++++++++ src/clj/com/rpl/specter/transient.cljx | 79 ++++++++++++++++++++++++++ 2 files changed, 128 insertions(+) create mode 100644 src/clj/com/rpl/specter/transient.cljx diff --git a/src/clj/com/rpl/specter/impl.cljx b/src/clj/com/rpl/specter/impl.cljx index 023390c..f5d5704 100644 --- a/src/clj/com/rpl/specter/impl.cljx +++ b/src/clj/com/rpl/specter/impl.cljx @@ -757,6 +757,55 @@ (next-fn structure) )) +(deftype TransientEndNavigator []) + +(extend-protocol p/Navigator + TransientEndNavigator + (select* [this structure next-fn] + []) + (transform* [this structure next-fn] + (let [res (next-fn [])] + (reduce conj! structure res)))) + +(deftype TransientLastNavigator []) + +(extend-protocol p/Navigator + TransientLastNavigator + (select* [this structure next-fn] + (next-fn (nth structure (dec (count structure))))) + (transform* [this structure next-fn] + (let [i (dec (count structure))] + (assoc! structure i (next-fn (nth structure i)))))) + +#+clj +(defn transient-all-select + [structure next-fn] + (into [] (r/mapcat #(next-fn (nth structure %)) + (range (count structure))))) + +#+cljs +(defn transient-all-select + [structure next-fn] + (into [] + (r/mapcat #(next-fn (nth structure %))) + (range (count structure)))) + +(defn transient-all-transform! + [structure next-fn] + (reduce (fn [structure i] + (assoc! structure i (next-fn (nth structure i)))) + structure + (range (count structure)))) + +(deftype TransientAllNavigator []) + +(extend-protocol p/Navigator + TransientAllNavigator + (select* [this structure next-fn] + (transient-all-select structure next-fn)) + (transform* [this structure next-fn] + (transient-all-transform! structure next-fn))) + (defn extract-basic-filter-fn [path] (cond (fn? path) path diff --git a/src/clj/com/rpl/specter/transient.cljx b/src/clj/com/rpl/specter/transient.cljx new file mode 100644 index 0000000..db0d1cd --- /dev/null +++ b/src/clj/com/rpl/specter/transient.cljx @@ -0,0 +1,79 @@ +(ns com.rpl.specter.transient + #+cljs + (:require-macros [com.rpl.specter.macros + :refer + [defnav + defpathedfn]]) + (:use #+clj + [com.rpl.specter.macros :only + [defnav + defpathedfn]]) + (:require [com.rpl.specter.impl :as i] + [com.rpl.specter :refer [subselect]])) + +(defnav + ^{:doc "Navigates to the specified key of a transient collection, + navigating to nil if it doesn't exist."} + keypath! + [key] + (select* [this structure next-fn] + (next-fn (get structure key))) + (transform* [this structure next-fn] + (assoc! structure key (next-fn (get structure key))))) + +(def END! + "Navigates to an empty (persistent) vector at the end of a transient vector." + (i/comp-paths* [(i/->TransientEndNavigator)])) + +(def FIRST! + "Navigates to the first element of a transient vector." + (keypath! 0)) + +(def LAST! + "Navigates to the last element of a transient vector." + (i/comp-paths* [(i/->TransientLastNavigator)])) + +(def ALL! + ;; TODO: only works on transient vectors, not sets / maps; need a + ;; different name? + "Navigates to each element of a transient vector." + (i/comp-paths* [(i/->TransientAllNavigator)])) + +(defpathedfn filterer! + "Navigates to a view of the current transient vector that only + contains elements that match the given path." + [& path] + (subselect ALL! (selected? path))) + +(defn- select-keys-from-transient-map + "Selects keys from transient map, because built-in select-keys uses + `find` which is unsupported." + [m m-keys] + (loop [result {} + m-keys m-keys] + (if-not (seq m-keys) + result + (let [k (first m-keys) + ;; support Clojure 1.6 where contains? is broken on transients + item (get m k ::not-found)] + (recur (if-not (identical? item ::not-found) + (assoc result k item) + result) + (rest m-keys)))))) + +(defnav + ^{:doc "Navigates to the specified persistent submap of a transient map."} + submap! + [m-keys] + (select* [this structure next-fn] + (select-keys-from-transient-map structure m-keys)) + (transform* [this structure next-fn] + (let [selected (select-keys-from-transient-map structure m-keys) + res (next-fn selected)] + (as-> structure % + (reduce (fn [m k] + (dissoc! m k)) + % m-keys) + (reduce (fn [m [k v]] + (assoc! m k v)) + % res))))) From 067ce9edee0ef4b73a264a048393517629f0a8b7 Mon Sep 17 00:00:00 2001 From: Alex Engelberg Date: Sun, 5 Jun 2016 11:10:40 -0700 Subject: [PATCH 02/21] Remove ALL! and filterer! --- src/clj/com/rpl/specter/impl.cljx | 9 --------- src/clj/com/rpl/specter/transient.cljx | 14 +------------- 2 files changed, 1 insertion(+), 22 deletions(-) diff --git a/src/clj/com/rpl/specter/impl.cljx b/src/clj/com/rpl/specter/impl.cljx index f5d5704..21f7b1f 100644 --- a/src/clj/com/rpl/specter/impl.cljx +++ b/src/clj/com/rpl/specter/impl.cljx @@ -797,15 +797,6 @@ structure (range (count structure)))) -(deftype TransientAllNavigator []) - -(extend-protocol p/Navigator - TransientAllNavigator - (select* [this structure next-fn] - (transient-all-select structure next-fn)) - (transform* [this structure next-fn] - (transient-all-transform! structure next-fn))) - (defn extract-basic-filter-fn [path] (cond (fn? path) path diff --git a/src/clj/com/rpl/specter/transient.cljx b/src/clj/com/rpl/specter/transient.cljx index db0d1cd..6cde71f 100644 --- a/src/clj/com/rpl/specter/transient.cljx +++ b/src/clj/com/rpl/specter/transient.cljx @@ -9,7 +9,7 @@ [defnav defpathedfn]]) (:require [com.rpl.specter.impl :as i] - [com.rpl.specter :refer [subselect]])) + [com.rpl.specter :refer [subselect selected?]])) (defnav ^{:doc "Navigates to the specified key of a transient collection, @@ -33,18 +33,6 @@ "Navigates to the last element of a transient vector." (i/comp-paths* [(i/->TransientLastNavigator)])) -(def ALL! - ;; TODO: only works on transient vectors, not sets / maps; need a - ;; different name? - "Navigates to each element of a transient vector." - (i/comp-paths* [(i/->TransientAllNavigator)])) - -(defpathedfn filterer! - "Navigates to a view of the current transient vector that only - contains elements that match the given path." - [& path] - (subselect ALL! (selected? path))) - (defn- select-keys-from-transient-map "Selects keys from transient map, because built-in select-keys uses `find` which is unsupported." From cb0dc261cf31d41f69465bacb082f03f2834c006 Mon Sep 17 00:00:00 2001 From: Alex Engelberg Date: Sun, 5 Jun 2016 21:38:14 -0700 Subject: [PATCH 03/21] Add tests for transients, fix transient navigators based on test failures --- src/clj/com/rpl/specter/impl.cljx | 23 +++++++++--------- src/clj/com/rpl/specter/transient.cljx | 23 +++++++++++++++--- test/com/rpl/specter/core_test.cljx | 32 ++++++++++++++++++++++++++ 3 files changed, 64 insertions(+), 14 deletions(-) diff --git a/src/clj/com/rpl/specter/impl.cljx b/src/clj/com/rpl/specter/impl.cljx index 21f7b1f..26cbf4f 100644 --- a/src/clj/com/rpl/specter/impl.cljx +++ b/src/clj/com/rpl/specter/impl.cljx @@ -466,6 +466,14 @@ (defn vec-count [v] (count v)) +#+clj +(defn transient-vec-count [^clojure.lang.ITransientVector v] + (.count v)) + +#+cljs +(defn transient-vec-count [v] + (count v)) + (extend-protocol UpdateExtremes #+clj clojure.lang.PersistentVector #+cljs cljs.core/PersistentVector (update-first [v afn] @@ -505,6 +513,9 @@ #+clj clojure.lang.IPersistentVector #+cljs cljs.core/PersistentVector (fast-empty? [v] (= 0 (vec-count v))) + #+clj clojure.lang.ITransientVector #+cljs cljs.core/TransientVector + (fast-empty? [v] + (= 0 (transient-vec-count v))) #+clj Object #+cljs default (fast-empty? [s] (empty? s)) @@ -762,21 +773,11 @@ (extend-protocol p/Navigator TransientEndNavigator (select* [this structure next-fn] - []) + (next-fn [])) (transform* [this structure next-fn] (let [res (next-fn [])] (reduce conj! structure res)))) -(deftype TransientLastNavigator []) - -(extend-protocol p/Navigator - TransientLastNavigator - (select* [this structure next-fn] - (next-fn (nth structure (dec (count structure))))) - (transform* [this structure next-fn] - (let [i (dec (count structure))] - (assoc! structure i (next-fn (nth structure i)))))) - #+clj (defn transient-all-select [structure next-fn] diff --git a/src/clj/com/rpl/specter/transient.cljx b/src/clj/com/rpl/specter/transient.cljx index 6cde71f..304db07 100644 --- a/src/clj/com/rpl/specter/transient.cljx +++ b/src/clj/com/rpl/specter/transient.cljx @@ -25,13 +25,30 @@ "Navigates to an empty (persistent) vector at the end of a transient vector." (i/comp-paths* [(i/->TransientEndNavigator)])) +(defn- t-get-first + [tv] + (nth tv 0)) + +(defn- t-get-last + [tv] + (nth tv (dec (i/transient-vec-count tv)))) + +(defn- t-update-first + [tv next-fn] + (assoc! tv 0 (next-fn (nth tv 0)))) + +(defn- t-update-last + [tv next-fn] + (let [i (dec (i/transient-vec-count tv))] + (assoc! tv i (next-fn (nth tv i))))) + (def FIRST! "Navigates to the first element of a transient vector." - (keypath! 0)) + (i/->PosNavigator t-get-first t-update-first)) (def LAST! "Navigates to the last element of a transient vector." - (i/comp-paths* [(i/->TransientLastNavigator)])) + (i/->PosNavigator t-get-last t-update-last)) (defn- select-keys-from-transient-map "Selects keys from transient map, because built-in select-keys uses @@ -54,7 +71,7 @@ submap! [m-keys] (select* [this structure next-fn] - (select-keys-from-transient-map structure m-keys)) + (next-fn (select-keys-from-transient-map structure m-keys))) (transform* [this structure next-fn] (let [selected (select-keys-from-transient-map structure m-keys) res (next-fn selected)] diff --git a/test/com/rpl/specter/core_test.cljx b/test/com/rpl/specter/core_test.cljx index cf0fbb1..5e2d72b 100644 --- a/test/com/rpl/specter/core_test.cljx +++ b/test/com/rpl/specter/core_test.cljx @@ -25,6 +25,7 @@ #+cljs [cljs.test.check.generators :as gen] #+cljs [cljs.test.check.properties :as prop :include-macros true] [com.rpl.specter :as s] + [com.rpl.specter.transient :as t] [clojure.set :as set])) ;;TODO: @@ -1036,3 +1037,34 @@ (is (= 2 (key e))) (is (= 4 (val e))) )) + +(defspec transient-vector-test + (for-all+ + [v (gen/vector (limit-size 5 gen/int))] + (every? identity + (for [[path transient-path f] + [[s/FIRST t/FIRST! (fnil inc 0)] ;; fnil in case vector is empty + [s/LAST t/LAST! (fnil inc 0)] + [(s/keypath 0) (t/keypath! 0) (fnil inc 0)] + [s/END t/END! #(conj % 1 2 3)]]] + (and (= (s/transform* path f v) + (persistent! (s/transform* transient-path f (transient v)))) + (= (s/select* path v) + (s/select* transient-path (transient v)))))))) + +(defspec transient-map-test + (for-all+ + [m (gen/not-empty (gen/map gen/keyword gen/int)) + new-key gen/keyword] + (let [existing-key (first (keys m))] + (every? identity + (for [[path transient-path f] + [[(s/keypath existing-key) (t/keypath! existing-key) inc] + [(s/keypath new-key) (t/keypath! new-key) (constantly 3)] + [(s/submap [existing-key new-key]) + (t/submap! [existing-key new-key]) + (constantly {new-key 1234})]]] + (and (= (s/transform* path f m) + (persistent! (s/transform* transient-path f (transient m)))) + (= (s/select* path m) + (s/select* transient-path (transient m))))))))) From 25ba21d9ee42cefbde0b0d49a1605285cf03fb66 Mon Sep 17 00:00:00 2001 From: Alex Engelberg Date: Sun, 5 Jun 2016 21:47:44 -0700 Subject: [PATCH 04/21] Remove no-longer-used transient-all-select|transform --- src/clj/com/rpl/specter/impl.cljx | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/src/clj/com/rpl/specter/impl.cljx b/src/clj/com/rpl/specter/impl.cljx index 26cbf4f..af1f7f3 100644 --- a/src/clj/com/rpl/specter/impl.cljx +++ b/src/clj/com/rpl/specter/impl.cljx @@ -778,26 +778,6 @@ (let [res (next-fn [])] (reduce conj! structure res)))) -#+clj -(defn transient-all-select - [structure next-fn] - (into [] (r/mapcat #(next-fn (nth structure %)) - (range (count structure))))) - -#+cljs -(defn transient-all-select - [structure next-fn] - (into [] - (r/mapcat #(next-fn (nth structure %))) - (range (count structure)))) - -(defn transient-all-transform! - [structure next-fn] - (reduce (fn [structure i] - (assoc! structure i (next-fn (nth structure i)))) - structure - (range (count structure)))) - (defn extract-basic-filter-fn [path] (cond (fn? path) path From 399e5661f115c49154efde8edaae0ff386720468 Mon Sep 17 00:00:00 2001 From: Alex Engelberg Date: Sun, 5 Jun 2016 22:11:34 -0700 Subject: [PATCH 05/21] The (identical?) trick doesn't work in cljs, but select-keys does --- src/clj/com/rpl/specter/transient.cljx | 7 +++++++ test/com/rpl/specter/core_test.cljx | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/clj/com/rpl/specter/transient.cljx b/src/clj/com/rpl/specter/transient.cljx index 304db07..ef15d54 100644 --- a/src/clj/com/rpl/specter/transient.cljx +++ b/src/clj/com/rpl/specter/transient.cljx @@ -50,6 +50,7 @@ "Navigates to the last element of a transient vector." (i/->PosNavigator t-get-last t-update-last)) +#+clj (defn- select-keys-from-transient-map "Selects keys from transient map, because built-in select-keys uses `find` which is unsupported." @@ -66,6 +67,12 @@ result) (rest m-keys)))))) +#+cljs +(defn- select-keys-from-transient-map + "Uses select-keys on a transient map." + [m m-keys] + (select-keys m m-keys)) + (defnav ^{:doc "Navigates to the specified persistent submap of a transient map."} submap! diff --git a/test/com/rpl/specter/core_test.cljx b/test/com/rpl/specter/core_test.cljx index 5e2d72b..225fd79 100644 --- a/test/com/rpl/specter/core_test.cljx +++ b/test/com/rpl/specter/core_test.cljx @@ -1054,7 +1054,7 @@ (defspec transient-map-test (for-all+ - [m (gen/not-empty (gen/map gen/keyword gen/int)) + [m (limit-size 5 (gen/not-empty (gen/map gen/keyword gen/int))) new-key gen/keyword] (let [existing-key (first (keys m))] (every? identity From ae98aa48bacc9fef716d657816a836b039f9087f Mon Sep 17 00:00:00 2001 From: Nathan Marz Date: Mon, 6 Jun 2016 16:03:08 -0400 Subject: [PATCH 06/21] add specialized MAP-VALS navigator to circumvent the unavoidable overhead of [ALL LAST] --- scripts/benchmarks.clj | 2 ++ src/clj/com/rpl/specter.cljx | 8 ++++++ src/clj/com/rpl/specter/impl.cljx | 43 +++++++++++++++++++++++++++++ test/com/rpl/specter/core_test.cljx | 10 ++++--- 4 files changed, 59 insertions(+), 4 deletions(-) diff --git a/scripts/benchmarks.clj b/scripts/benchmarks.clj index 6f3ac43..e93357e 100644 --- a/scripts/benchmarks.clj +++ b/scripts/benchmarks.clj @@ -106,6 +106,7 @@ (reduce-kv (fn [m k v] (assoc m k (inc v))) {} data) (manual-similar-reduce-kv data) (transform [ALL LAST] inc data) + (transform MAP-VALS inc data) )) (let [data (->> (for [i (range 1000)] [i i]) (into {}))] @@ -114,6 +115,7 @@ (reduce-kv (fn [m k v] (assoc m k (inc v))) {} data) (manual-similar-reduce-kv data) (transform [ALL LAST] inc data) + (transform MAP-VALS inc data) )) diff --git a/src/clj/com/rpl/specter.cljx b/src/clj/com/rpl/specter.cljx index 756d127..9d7bac2 100644 --- a/src/clj/com/rpl/specter.cljx +++ b/src/clj/com/rpl/specter.cljx @@ -163,6 +163,14 @@ ALL (comp-paths (i/->AllNavigator))) +(defnav MAP-VALS [] + (select* [this structure next-fn] + (doall (mapcat next-fn (vals structure)))) + (transform* [this structure next-fn] + (i/map-vals-transform structure next-fn) + )) + + (def VAL (i/->ValCollect)) (def diff --git a/src/clj/com/rpl/specter/impl.cljx b/src/clj/com/rpl/specter/impl.cljx index 196a29d..c1db2ed 100644 --- a/src/clj/com/rpl/specter/impl.cljx +++ b/src/clj/com/rpl/specter/impl.cljx @@ -702,6 +702,49 @@ (transform* [this structure next-fn] (all-transform structure next-fn))) +(defprotocol MapValsTransformProtocol + (map-vals-transform [structure next-fn])) + +(defn map-vals-non-transient-transform [structure empty-map next-fn] + (reduce-kv + (fn [m k v] + (assoc m k (next-fn v))) + empty-map + structure)) + +(extend-protocol MapValsTransformProtocol + #+clj clojure.lang.PersistentArrayMap #+cljs cljs.core/PersistentArrayMap + (map-vals-transform [structure next-fn] + (map-vals-non-transient-transform structure {} next-fn) + ) + + #+clj clojure.lang.PersistentTreeMap #+cljs cljs.core/PersistentTreeMap + (map-vals-transform [structure next-fn] + (map-vals-non-transient-transform structure (sorted-map) next-fn) + ) + + #+clj clojure.lang.PersistentHashMap #+cljs cljs.core/PersistentHashMap + (map-vals-transform [structure next-fn] + (persistent! + (reduce-kv + (fn [m k v] + (assoc! m k (next-fn v))) + (transient + #+clj clojure.lang.PersistentHashMap/EMPTY #+cljs cljs.core.PersistentHashMap.EMPTY + ) + structure + ))) + + #+clj Object #+cljs default + (map-vals-transform [structure next-fn] + (reduce-kv + (fn [m k v] + (assoc m k (next-fn v))) + (empty structure) + structure)) + ) + + (deftype ValCollect []) (extend-protocol p/Collector diff --git a/test/com/rpl/specter/core_test.cljx b/test/com/rpl/specter/core_test.cljx index cf0fbb1..64acbc4 100644 --- a/test/com/rpl/specter/core_test.cljx +++ b/test/com/rpl/specter/core_test.cljx @@ -64,8 +64,9 @@ (defspec select-all-on-map (for-all+ - [m (limit-size 5 (gen/map gen/keyword gen/int))] - (= (select [s/ALL s/LAST] m) + [m (limit-size 5 (gen/map gen/keyword gen/int)) + p (gen/elements [s/MAP-VALS [s/ALL s/LAST]])] + (= (select p m) (for [[k v] m] v)) )) @@ -81,8 +82,9 @@ (defspec transform-all-on-map (for-all+ - [m (limit-size 5 (gen/map gen/keyword gen/int))] - (= (transform [s/ALL s/LAST] inc m) + [m (limit-size 5 (gen/map gen/keyword gen/int)) + p (gen/elements [s/MAP-VALS [s/ALL s/LAST]])] + (= (transform p inc m) (into {} (for [[k v] m] [k (inc v)])) ))) From 4379a3dc9c328a71d78e5ce5180e55dac2170a21 Mon Sep 17 00:00:00 2001 From: Nathan Marz Date: Mon, 6 Jun 2016 16:10:47 -0400 Subject: [PATCH 07/21] update changelog --- CHANGES.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGES.md b/CHANGES.md index 420e393..f20b99c 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -3,6 +3,7 @@ * Huge performance improvement for ALL transform on maps and vectors * Significant performance improvements for FIRST/LAST for vectors * Huge performance improvements for `if-path`, `cond-path`, `selected?`, and `not-selected?`, especially for condition path containing only static functions +* Added specialized MAP-VALS navigator that is twice as fast as using [ALL LAST] * Eliminated compiler warnings for ClojureScript version * Dropped support for Clojurescript below v1.7.10 * Added :notpath metadata to signify pathedfn arguments that should be treated as regular arguments during inline factoring. If one of these arguments is not a static var reference or non-collection value, the path will not factor. From 81ec559e69bbdbb0bd3cbc135c571cbea2158ec5 Mon Sep 17 00:00:00 2001 From: Nathan Marz Date: Tue, 7 Jun 2016 10:31:07 -0400 Subject: [PATCH 08/21] docstring for MAP-VALS --- src/clj/com/rpl/specter.cljx | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/clj/com/rpl/specter.cljx b/src/clj/com/rpl/specter.cljx index 9d7bac2..b655a15 100644 --- a/src/clj/com/rpl/specter.cljx +++ b/src/clj/com/rpl/specter.cljx @@ -163,9 +163,15 @@ ALL (comp-paths (i/->AllNavigator))) -(defnav MAP-VALS [] +(defnav + ^{:doc "Navigate to each value of the map. This is more efficient than + navigating via [ALL LAST]"} + MAP-VALS + [] (select* [this structure next-fn] - (doall (mapcat next-fn (vals structure)))) + (doseqres NONE [v (vals structure)] + (next-fn v) + )) (transform* [this structure next-fn] (i/map-vals-transform structure next-fn) )) From 205b6a13193092ad990df22efb30575f4ac01545 Mon Sep 17 00:00:00 2001 From: Nathan Marz Date: Tue, 7 Jun 2016 10:51:08 -0400 Subject: [PATCH 09/21] fix MAP-VALS --- src/clj/com/rpl/specter.cljx | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/clj/com/rpl/specter.cljx b/src/clj/com/rpl/specter.cljx index b655a15..dc794c9 100644 --- a/src/clj/com/rpl/specter.cljx +++ b/src/clj/com/rpl/specter.cljx @@ -169,9 +169,7 @@ MAP-VALS [] (select* [this structure next-fn] - (doseqres NONE [v (vals structure)] - (next-fn v) - )) + (doall (mapcat next-fn (vals structure)))) (transform* [this structure next-fn] (i/map-vals-transform structure next-fn) )) From c28245b4200c5ff8b4e51361b49fa1f50cd86b33 Mon Sep 17 00:00:00 2001 From: Nathan Marz Date: Tue, 7 Jun 2016 14:40:31 -0400 Subject: [PATCH 10/21] add protocols ns to api docs --- project.clj | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/project.clj b/project.clj index 9f2ab74..a646ba8 100644 --- a/project.clj +++ b/project.clj @@ -14,7 +14,8 @@ :codox {:source-paths ["target/classes" "src/clj"] :namespaces [com.rpl.specter com.rpl.specter.macros - com.rpl.specter.zipper] + com.rpl.specter.zipper + com.rpl.specter.protocols] :source-uri {#"target/classes" "https://github.com/nathanmarz/specter/tree/{version}/src/clj/{classpath}x#L{line}" #".*" "https://github.com/nathanmarz/specter/tree/{version}/src/clj/{classpath}#L{line}" From 5161f6dfbfa5d1919170b351a5d1723de2c51be0 Mon Sep 17 00:00:00 2001 From: Nathan Marz Date: Tue, 7 Jun 2016 16:07:01 -0400 Subject: [PATCH 11/21] optimize END for vectors --- CHANGES.md | 1 + scripts/benchmarks.clj | 8 ++++++-- src/clj/com/rpl/specter.cljx | 22 +++++++++++++++++----- src/clj/com/rpl/specter/impl.cljx | 24 ++++++++++++++++++++++++ 4 files changed, 48 insertions(+), 7 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index f20b99c..71152b0 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -3,6 +3,7 @@ * Huge performance improvement for ALL transform on maps and vectors * Significant performance improvements for FIRST/LAST for vectors * Huge performance improvements for `if-path`, `cond-path`, `selected?`, and `not-selected?`, especially for condition path containing only static functions +* Huge performance improvement for `END` on vectors * Added specialized MAP-VALS navigator that is twice as fast as using [ALL LAST] * Eliminated compiler warnings for ClojureScript version * Dropped support for Clojurescript below v1.7.10 diff --git a/scripts/benchmarks.clj b/scripts/benchmarks.clj index e93357e..35484b0 100644 --- a/scripts/benchmarks.clj +++ b/scripts/benchmarks.clj @@ -49,8 +49,6 @@ (println "\n********************************\n") ))) - - (let [data {:a {:b {:c 1}}} p (comp-paths :a :b :c)] (run-benchmark "get value in nested map" 10000000 @@ -87,6 +85,12 @@ (transform ALL inc data) )) +(let [v (vec (range 1000))] + (run-benchmark "END on large vector" + 5000000 + (setval END [1] v) + (reduce conj v [1]) + (conj v 1))) (defn- update-pair [[k v]] [k (inc v)]) diff --git a/src/clj/com/rpl/specter.cljx b/src/clj/com/rpl/specter.cljx index dc794c9..d55b8e9 100644 --- a/src/clj/com/rpl/specter.cljx +++ b/src/clj/com/rpl/specter.cljx @@ -229,16 +229,28 @@ (reverse (i/matching-ranges structure pred)) ))) -(def +(defnav ^{:doc "Navigate to the empty subsequence before the first element of the collection."} BEGINNING - (srange 0 0)) + [] + (select* [this structure next-fn] + (next-fn [])) + (transform* [this structure next-fn] + (let [to-prepend (next-fn [])] + (i/prepend-all structure to-prepend) + ))) -(def +(defnav ^{:doc "Navigate to the empty subsequence after the last element of the collection."} END - (srange-dynamic count count)) - + [] + (select* [this structure next-fn] + (next-fn [])) + (transform* [this structure next-fn] + (let [to-append (next-fn [])] + (i/append-all structure to-append) + ))) + (defnav ^{:doc "Navigates to the specified subset (by taking an intersection). In a transform, that subset in the original set is changed to the diff --git a/src/clj/com/rpl/specter/impl.cljx b/src/clj/com/rpl/specter/impl.cljx index c1db2ed..b401e79 100644 --- a/src/clj/com/rpl/specter/impl.cljx +++ b/src/clj/com/rpl/specter/impl.cljx @@ -448,6 +448,30 @@ (defn- append [coll elem] (-> coll vec (conj elem))) +(defprotocol AddExtremes + (append-all [structure elements]) + (prepend-all [structure elements])) + +(extend-protocol AddExtremes + #+clj clojure.lang.PersistentVector #+cljs cljs.core/PersistentVector + (append-all [structure elements] + (reduce conj structure elements)) + (prepend-all [structure elements] + (let [ret (transient [])] + (as-> ret <> + (reduce conj! <> elements) + (reduce conj! <> structure) + (persistent! <>) + ))) + + #+clj Object #+cljs default + (append-all [structure elements] + (concat structure elements)) + (prepend-all [structure elements] + (concat elements structure)) + ) + + (defprotocol UpdateExtremes (update-first [s afn]) (update-last [s afn])) From bafe10036fb3dae27e507db7cc2013bf33593d7a Mon Sep 17 00:00:00 2001 From: Alex Engelberg Date: Tue, 7 Jun 2016 23:16:23 -0700 Subject: [PATCH 12/21] Add benchmarks to test transient navigators --- scripts/benchmarks.clj | 44 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/scripts/benchmarks.clj b/scripts/benchmarks.clj index 0516f52..61aa268 100644 --- a/scripts/benchmarks.clj +++ b/scripts/benchmarks.clj @@ -1,6 +1,7 @@ (ns com.rpl.specter.benchmarks (:use [com.rpl.specter] [com.rpl.specter macros] + [com.rpl.specter.transient] [com.rpl.specter.impl :only [benchmark]]) (:require [clojure.walk :as walk])) @@ -141,3 +142,46 @@ (tree-value-transform (fn [e] (if (even? e) (inc e) e)) data) )) +(run-benchmark "transient comparison: building up vectors" + 6 + 10000 + (reduce (fn [v i] (conj v i)) [] (range 1000)) + (reduce (fn [v i] (conj! v i)) (transient []) (range 1000)) + ;; uncomment this when END is fast + #_(reduce (fn [v i] (setval [END] [i] v)) [] (range 1000)) + (setval [END!] (range 1000) (transient [])) + (reduce (fn [v i] (setval [END!] [i] v)) (transient []) (range 1000))) + +(let [data (vec (range 1000)) + tdata (transient data)] + (run-benchmark "transient comparison: assoc'ing in vectors" + 6 + 500000 + (assoc data (rand-int 1000) 0) + (assoc! tdata (rand-int 1000) 0) + (setval [(keypath (rand-int 1000))] 0 data) + (setval [(keypath! (rand-int 1000))] 0 tdata))) + +(let [data (into {} (for [k (range 1000)] + [k (rand)])) + tdata (transient data)] + (run-benchmark "transient comparison: assoc'ing in maps" + 6 + 500000 + (assoc data (rand-int 1000) 0) + (assoc! tdata (rand-int 1000) 0) + (setval [(keypath (rand-int 1000))] 0 data) + (setval [(keypath! (rand-int 1000))] 0 tdata))) + +(defn modify-submap + [m] + (assoc m 0 1 458 89)) + +(let [data (into {} (for [k (range 1000)] + [k (rand)])) + tdata (transient data)] + (run-benchmark "transient comparison: submap" + 6 + 300000 + (transform [(submap [(rand-int 1000)])] modify-submap data) + (transform [(submap! [(rand-int 1000)])] modify-submap tdata))) From 2147584dca7c2f227cbd1ae4c157af402c8630c9 Mon Sep 17 00:00:00 2001 From: Alex Engelberg Date: Tue, 7 Jun 2016 23:16:54 -0700 Subject: [PATCH 13/21] Change reduce to reduce-kv --- src/clj/com/rpl/specter/transient.cljx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/clj/com/rpl/specter/transient.cljx b/src/clj/com/rpl/specter/transient.cljx index ef15d54..774e19f 100644 --- a/src/clj/com/rpl/specter/transient.cljx +++ b/src/clj/com/rpl/specter/transient.cljx @@ -86,6 +86,6 @@ (reduce (fn [m k] (dissoc! m k)) % m-keys) - (reduce (fn [m [k v]] - (assoc! m k v)) - % res))))) + (reduce-kv (fn [m k v] + (assoc! m k v)) + % res))))) From 82321d73707fbbd0421de1bd27fe87d0a0a1d95b Mon Sep 17 00:00:00 2001 From: Nathan Marz Date: Wed, 8 Jun 2016 06:02:41 -0400 Subject: [PATCH 14/21] isolate desired operations to test in transient benchmarks and make the comparisons work on identical data, add transient namespace for doc generation --- project.clj | 3 ++- scripts/benchmarks.clj | 47 +++++++++++++++++++++--------------------- 2 files changed, 25 insertions(+), 25 deletions(-) diff --git a/project.clj b/project.clj index a646ba8..fd2966b 100644 --- a/project.clj +++ b/project.clj @@ -15,7 +15,8 @@ :namespaces [com.rpl.specter com.rpl.specter.macros com.rpl.specter.zipper - com.rpl.specter.protocols] + com.rpl.specter.protocols + com.rpl.specter.transient] :source-uri {#"target/classes" "https://github.com/nathanmarz/specter/tree/{version}/src/clj/{classpath}x#L{line}" #".*" "https://github.com/nathanmarz/specter/tree/{version}/src/clj/{classpath}#L{line}" diff --git a/scripts/benchmarks.clj b/scripts/benchmarks.clj index e806dc0..fafbbd1 100644 --- a/scripts/benchmarks.clj +++ b/scripts/benchmarks.clj @@ -154,36 +154,36 @@ (tree-value-transform (fn [e] (if (even? e) (inc e) e)) data) )) -(run-benchmark "transient comparison: building up vectors" - 6 - 10000 - (reduce (fn [v i] (conj v i)) [] (range 1000)) - (reduce (fn [v i] (conj! v i)) (transient []) (range 1000)) - ;; uncomment this when END is fast - #_(reduce (fn [v i] (setval [END] [i] v)) [] (range 1000)) - (setval [END!] (range 1000) (transient [])) - (reduce (fn [v i] (setval [END!] [i] v)) (transient []) (range 1000))) +(let [toappend (range 1000)] + (run-benchmark "transient comparison: building up vectors" + 10000 + (reduce (fn [v i] (conj v i)) [] toappend) + (reduce (fn [v i] (conj! v i)) (transient []) toappend) + (setval END toappend []) + (setval END! toappend (transient [])))) (let [data (vec (range 1000)) - tdata (transient data)] + tdata (transient data) + tdata2 (transient data) + idx 600] (run-benchmark "transient comparison: assoc'ing in vectors" - 6 500000 - (assoc data (rand-int 1000) 0) - (assoc! tdata (rand-int 1000) 0) - (setval [(keypath (rand-int 1000))] 0 data) - (setval [(keypath! (rand-int 1000))] 0 tdata))) + (assoc data idx 0) + (assoc! tdata idx 0) + (setval (keypath idx) 0 data) + (setval (keypath! idx) 0 tdata2))) (let [data (into {} (for [k (range 1000)] [k (rand)])) - tdata (transient data)] + tdata (transient data) + tdata2 (transient data) + idx 600] (run-benchmark "transient comparison: assoc'ing in maps" - 6 500000 - (assoc data (rand-int 1000) 0) - (assoc! tdata (rand-int 1000) 0) - (setval [(keypath (rand-int 1000))] 0 data) - (setval [(keypath! (rand-int 1000))] 0 tdata))) + (assoc data idx 0) + (assoc! tdata idx 0) + (setval (keypath idx) 0 data) + (setval (keypath! idx) 0 tdata2))) (defn modify-submap [m] @@ -193,7 +193,6 @@ [k (rand)])) tdata (transient data)] (run-benchmark "transient comparison: submap" - 6 300000 - (transform [(submap [(rand-int 1000)])] modify-submap data) - (transform [(submap! [(rand-int 1000)])] modify-submap tdata))) + (transform (submap [600 700]) modify-submap data) + (transform (submap! [600 700]) modify-submap tdata))) From 4cdc7a47a305d98aef2767df2cd5b6266fc4ec6b Mon Sep 17 00:00:00 2001 From: Nathan Marz Date: Wed, 8 Jun 2016 06:09:24 -0400 Subject: [PATCH 15/21] updated changelog --- CHANGES.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGES.md b/CHANGES.md index 71152b0..00e2b1b 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,5 +1,6 @@ ## 0.11.1 (unreleased) * More efficient inline caching for Clojure version, benchmarks show inline caching within 5% of manually precompiled code for all cases +* Added navigators for transients in com.rpl.specter.transient namespace (thanks @aaengelberg) * Huge performance improvement for ALL transform on maps and vectors * Significant performance improvements for FIRST/LAST for vectors * Huge performance improvements for `if-path`, `cond-path`, `selected?`, and `not-selected?`, especially for condition path containing only static functions From 8231cd654f5fe619aa848ff5f2278feb6f833b56 Mon Sep 17 00:00:00 2001 From: Nathan Marz Date: Wed, 8 Jun 2016 06:11:21 -0400 Subject: [PATCH 16/21] benchmark keypath alongside direct keyword navigation --- scripts/benchmarks.clj | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/benchmarks.clj b/scripts/benchmarks.clj index fafbbd1..52b527f 100644 --- a/scripts/benchmarks.clj +++ b/scripts/benchmarks.clj @@ -55,6 +55,7 @@ (run-benchmark "get value in nested map" 10000000 (get-in data [:a :b :c]) (select [:a :b :c] data) + (select [(keypath :a) (keypath :b) (keypath :c)] data) (compiled-select p data) (-> data :a :b :c vector) ) From 50c176ba4825bf0370b7f2b67ca45eb314c7ef48 Mon Sep 17 00:00:00 2001 From: Nathan Marz Date: Wed, 8 Jun 2016 06:57:15 -0400 Subject: [PATCH 17/21] reinstate one-at-at-time vector append benchmark --- scripts/benchmarks.clj | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/scripts/benchmarks.clj b/scripts/benchmarks.clj index 52b527f..475413c 100644 --- a/scripts/benchmarks.clj +++ b/scripts/benchmarks.clj @@ -163,12 +163,21 @@ (setval END toappend []) (setval END! toappend (transient [])))) +(let [toappend (range 1000)] + (run-benchmark "transient comparison: building up vectors one at a time" + 10000 + (reduce (fn [v i] (conj v i)) [] toappend) + (reduce (fn [v i] (conj! v i)) (transient []) toappend) + (reduce (fn [v i] (setval END [i] v)) [] toappend) + (reduce (fn [v i] (setval END! [i] v)) (transient []) toappend) + )) + (let [data (vec (range 1000)) tdata (transient data) tdata2 (transient data) idx 600] (run-benchmark "transient comparison: assoc'ing in vectors" - 500000 + 2500000 (assoc data idx 0) (assoc! tdata idx 0) (setval (keypath idx) 0 data) @@ -180,7 +189,7 @@ tdata2 (transient data) idx 600] (run-benchmark "transient comparison: assoc'ing in maps" - 500000 + 2500000 (assoc data idx 0) (assoc! tdata idx 0) (setval (keypath idx) 0 data) From 3a9de5b70faacdc22791c5bd5b36be66119b774b Mon Sep 17 00:00:00 2001 From: Nathan Marz Date: Wed, 8 Jun 2016 06:57:32 -0400 Subject: [PATCH 18/21] 0.11.1 --- VERSION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VERSION b/VERSION index 9ba95d1..af88ba8 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.11.1-SNAPSHOT +0.11.1 From 839d92da1460eaa6ee62e0b1f07024c6fda82d67 Mon Sep 17 00:00:00 2001 From: Nathan Marz Date: Wed, 8 Jun 2016 07:01:17 -0400 Subject: [PATCH 19/21] update changelog --- CHANGES.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGES.md b/CHANGES.md index 00e2b1b..09f712e 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,4 +1,4 @@ -## 0.11.1 (unreleased) +## 0.11.1 * More efficient inline caching for Clojure version, benchmarks show inline caching within 5% of manually precompiled code for all cases * Added navigators for transients in com.rpl.specter.transient namespace (thanks @aaengelberg) * Huge performance improvement for ALL transform on maps and vectors From 85f14e839869d1c57327cf3a13d4a74a14385b8e Mon Sep 17 00:00:00 2001 From: Nathan Marz Date: Wed, 8 Jun 2016 07:06:04 -0400 Subject: [PATCH 20/21] update README --- README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 6041d4f..1c6ecd0 100644 --- a/README.md +++ b/README.md @@ -23,7 +23,8 @@ Specter's API is contained in three files: - [macros.clj](https://github.com/nathanmarz/specter/blob/master/src/clj/com/rpl/specter/macros.clj): This contains the core `select/transform/etc.` operations as well as macros for defining new navigators. - [specter.cljx](https://github.com/nathanmarz/specter/blob/master/src/clj/com/rpl/specter.cljx): This contains the build-in navigators and functional versions of `select/transform/etc.` -- [zippers.cljx](https://github.com/nathanmarz/specter/blob/master/src/clj/com/rpl/specter/zipper.cljx): This integrates zipper-based navigation into Specter. +- [transient.cljx](https://github.com/nathanmarz/specter/blob/master/src/clj/com/rpl/specter/transient.cljx): This contains navigators for transient collections. +- [zipper.cljx](https://github.com/nathanmarz/specter/blob/master/src/clj/com/rpl/specter/zipper.cljx): This integrates zipper-based navigation into Specter. # Questions? From 3d84d288d121342359ab212612a768aa28e13752 Mon Sep 17 00:00:00 2001 From: Nathan Marz Date: Wed, 8 Jun 2016 09:40:53 -0400 Subject: [PATCH 21/21] fix typo in readme --- CHANGES.md | 2 +- scripts/benchmarks.clj | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 09f712e..f0cc5fb 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -11,7 +11,7 @@ * Added :notpath metadata to signify pathedfn arguments that should be treated as regular arguments during inline factoring. If one of these arguments is not a static var reference or non-collection value, the path will not factor. * Bug fix: `transformed` transform-fn no longer factors into `pred` when an anonymous function during inline factoring * Bug fix: Fixed nil->val to not replace the val on `false` -* Bug fix: Eliminate reflection when using primitive paramaters in an inline cached path +* Bug fix: Eliminate reflection when using primitive parameters in an inline cached path ## 0.11.0 * New `path` macro does intelligent inline caching of the provided path. The path is factored into a static portion and into params which may change on each usage of the path (e.g. local parameters). The static part is factored and compiled on the first run-through, and then re-used for all subsequent invocations. As an example, `[ALL (keypath k)]` is factored into `[ALL keypath]`, which is compiled and cached, and `[k]`, which is provided on each execution. If it is not possible to precompile the path (e.g. [ALL some-local-variable]), nothing is cached and the path will be compiled on each run-through. diff --git a/scripts/benchmarks.clj b/scripts/benchmarks.clj index 475413c..4396209 100644 --- a/scripts/benchmarks.clj +++ b/scripts/benchmarks.clj @@ -31,7 +31,7 @@ (time-ms amt-per-iter afn)))) (defn compare-benchmark [amt-per-iter afn-map] - (let [results (transform [ALL LAST] + (let [results (transform MAP-VALS (fn [afn] (average-time-ms 8 amt-per-iter afn)) afn-map)