From b3e581f7371a96b8cb2098ce2e7049349508af4e Mon Sep 17 00:00:00 2001 From: Alex Engelberg Date: Sun, 5 Jun 2016 00:46:21 -0700 Subject: [PATCH 1/7] 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 2/7] 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 3/7] 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 4/7] 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 5/7] 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 bafe10036fb3dae27e507db7cc2013bf33593d7a Mon Sep 17 00:00:00 2001 From: Alex Engelberg Date: Tue, 7 Jun 2016 23:16:23 -0700 Subject: [PATCH 6/7] 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 7/7] 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)))))