From faaaedaa29a5c41af3397c238cd5e0103e972f47 Mon Sep 17 00:00:00 2001 From: Tommi Reiman Date: Sun, 26 Apr 2020 21:27:55 +0300 Subject: [PATCH 1/5] Enable quick creation of routers --- modules/reitit-core/src/reitit/core.cljc | 10 ++--- .../clj/reitit/router_creation_perf_test.clj | 43 +++++++++++++++++++ 2 files changed, 47 insertions(+), 6 deletions(-) create mode 100644 perf-test/clj/reitit/router_creation_perf_test.clj diff --git a/modules/reitit-core/src/reitit/core.cljc b/modules/reitit-core/src/reitit/core.cljc index a582badb..25bd3ef8 100644 --- a/modules/reitit-core/src/reitit/core.cljc +++ b/modules/reitit-core/src/reitit/core.cljc @@ -364,10 +364,10 @@ ([raw-routes] (router raw-routes {})) ([raw-routes opts] - (let [{:keys [router] :as opts} (merge (default-router-options) opts)] + (let [{:keys [router conflicts] :as opts} (merge (default-router-options) opts)] (try (let [routes (impl/resolve-routes raw-routes opts) - path-conflicting (impl/path-conflicting-routes routes opts) + path-conflicting (if-not (and router (not conflicts)) (impl/path-conflicting-routes routes opts)) name-conflicting (impl/name-conflicting-routes routes) compiled-routes (impl/compile-routes routes opts) wilds? (boolean (some (impl/->wild-route? opts) compiled-routes)) @@ -380,10 +380,8 @@ all-wilds? trie-router :else mixed-router)] - (when-let [conflicts (:conflicts opts)] - (when-let [conflict-report (impl/unresolved-conflicts - path-conflicting)] - (conflicts conflict-report))) + (when-let [conflict-report (and conflicts (impl/unresolved-conflicts path-conflicting))] + (conflicts conflict-report)) (when name-conflicting (exception/fail! :name-conflicts name-conflicting)) diff --git a/perf-test/clj/reitit/router_creation_perf_test.clj b/perf-test/clj/reitit/router_creation_perf_test.clj new file mode 100644 index 00000000..de9f4b23 --- /dev/null +++ b/perf-test/clj/reitit/router_creation_perf_test.clj @@ -0,0 +1,43 @@ +(ns reitit.router-creation-perf-test + (:require [reitit.perf-utils :refer [bench!]] + [reitit.core :as r] + [clojure.string :as str]) + (:import (java.util Random))) + +;; +;; start repl with `lein perf repl` +;; perf measured with the following setup: +;; +;; Model Name: MacBook Pro +;; Model Identifier: MacBookPro113 +;; Processor Name: Intel Core i7 +;; Processor Speed: 2,5 GHz +;; Number of Processors: 1 +;; Total Number of Cores: 4 +;; L2 Cache (per Core): 256 KB +;; L3 Cache: 6 MB +;; Memory: 16 GB +;; + +(defn random [^long seed] + (Random. seed)) + +(defn rand-str [^Random rnd len] + (apply str (take len (repeatedly #(char (+ (.nextInt rnd 26) 97)))))) + +(defn route [rnd] + (str/join "/" (repeatedly (+ 2 (.nextInt rnd 8)) (fn [] (rand-str rnd (.nextInt rnd 10)))))) + +(def hundred-routes + (let [rnd (random 1)] + (mapv (fn [n] [(route rnd) (keyword (str "route" n))]) (range 100)))) + +(defn bench-routers [] + ;; 104ms + (bench! "default" (r/router hundred-routes)) + + ;; 7ms + (bench! "linear" (r/router hundred-routes {:router r/linear-router, :conflicts nil}))) + +(comment + (bench-routers)) From f45f5859eb3330bda34f16661e9b85ba77f4ddc7 Mon Sep 17 00:00:00 2001 From: Tommi Reiman Date: Sun, 26 Apr 2020 21:28:04 +0300 Subject: [PATCH 2/5] Fix reflection warning --- modules/reitit-core/src/reitit/impl.cljc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/reitit-core/src/reitit/impl.cljc b/modules/reitit-core/src/reitit/impl.cljc index 2144d000..6dbb1954 100644 --- a/modules/reitit-core/src/reitit/impl.cljc +++ b/modules/reitit-core/src/reitit/impl.cljc @@ -179,7 +179,7 @@ (URLDecoder/decode (if (.contains ^String s "+") (.replace ^String s "+" "%2B") - s) + ^String s) "UTF-8")) :cljs (js/decodeURIComponent s)))) From 0d3a195cd86169fea29439ff08292147e760e996 Mon Sep 17 00:00:00 2001 From: Tommi Reiman Date: Sun, 26 Apr 2020 21:28:08 +0300 Subject: [PATCH 3/5] Format --- modules/reitit-core/src/reitit/impl.cljc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/reitit-core/src/reitit/impl.cljc b/modules/reitit-core/src/reitit/impl.cljc index 6dbb1954..3e58e48b 100644 --- a/modules/reitit-core/src/reitit/impl.cljc +++ b/modules/reitit-core/src/reitit/impl.cljc @@ -71,7 +71,7 @@ (defn resolve-routes [raw-routes {:keys [coerce] :as opts}] (cond->> (->> (walk raw-routes opts) (map-data merge-data)) - coerce (into [] (keep #(coerce % opts))))) + coerce (into [] (keep #(coerce % opts))))) (defn path-conflicting-routes [routes opts] (-> (into {} From b128a0f3db1b78e6790ab2bf98068a76124d5857 Mon Sep 17 00:00:00 2001 From: Tommi Reiman Date: Sun, 26 Apr 2020 22:04:14 +0300 Subject: [PATCH 4/5] Run path-conflicting just once for quarantine router --- modules/reitit-core/src/reitit/core.cljc | 4 ++-- .../clj/reitit/router_creation_perf_test.clj | 17 +++++++++++++++-- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/modules/reitit-core/src/reitit/core.cljc b/modules/reitit-core/src/reitit/core.cljc index 25bd3ef8..bcd54ac4 100644 --- a/modules/reitit-core/src/reitit/core.cljc +++ b/modules/reitit-core/src/reitit/core.cljc @@ -301,7 +301,7 @@ ([compiled-routes] (quarantine-router compiled-routes {})) ([compiled-routes opts] - (let [conflicting-paths (-> compiled-routes (impl/path-conflicting-routes opts) impl/conflicting-paths) + (let [conflicting-paths (impl/conflicting-paths (or (::path-conflicting opts) (impl/path-conflicting-routes compiled-routes opts))) conflicting? #(contains? conflicting-paths (first %)) {conflicting true, non-conflicting false} (group-by conflicting? compiled-routes) linear-router (linear-router conflicting opts) @@ -389,7 +389,7 @@ (when-let [validate (:validate opts)] (validate compiled-routes opts)) - (router compiled-routes opts)) + (router compiled-routes (assoc opts ::path-conflicting path-conflicting))) (catch #?(:clj Exception, :cljs js/Error) e (throw ((get opts :exception identity) e))))))) diff --git a/perf-test/clj/reitit/router_creation_perf_test.clj b/perf-test/clj/reitit/router_creation_perf_test.clj index de9f4b23..9349d128 100644 --- a/perf-test/clj/reitit/router_creation_perf_test.clj +++ b/perf-test/clj/reitit/router_creation_perf_test.clj @@ -1,5 +1,5 @@ (ns reitit.router-creation-perf-test - (:require [reitit.perf-utils :refer [bench!]] + (:require [reitit.perf-utils :refer [bench! suite]] [reitit.core :as r] [clojure.string :as str]) (:import (java.util Random))) @@ -32,12 +32,25 @@ (let [rnd (random 1)] (mapv (fn [n] [(route rnd) (keyword (str "route" n))]) (range 100)))) +(conj hundred-routes (last hundred-routes)) + + (defn bench-routers [] + + (suite "non-conflicting") + ;; 104ms (bench! "default" (r/router hundred-routes)) ;; 7ms - (bench! "linear" (r/router hundred-routes {:router r/linear-router, :conflicts nil}))) + (bench! "linear" (r/router hundred-routes {:router r/linear-router, :conflicts nil})) + + (suite "conflicting") + (let [routes (conj hundred-routes [(first (last hundred-routes)) ::route])] + + ;; 205ms + ;; 105ms (cache path-conflicts) + (bench! "default" (r/router routes {:conflicts nil})))) (comment (bench-routers)) From 1b0cc0a100a3c6d19a36c1927bc3b64697700b25 Mon Sep 17 00:00:00 2001 From: Tommi Reiman Date: Mon, 27 Apr 2020 08:38:27 +0300 Subject: [PATCH 5/5] Faster path conflict resolution, O(n2) -> O(n) --- modules/reitit-core/src/reitit/impl.cljc | 17 ++++++++------- modules/reitit-core/src/reitit/trie.cljc | 21 ++++++++++--------- .../clj/reitit/router_creation_perf_test.clj | 2 ++ 3 files changed, 22 insertions(+), 18 deletions(-) diff --git a/modules/reitit-core/src/reitit/impl.cljc b/modules/reitit-core/src/reitit/impl.cljc index 3e58e48b..cc902295 100644 --- a/modules/reitit-core/src/reitit/impl.cljc +++ b/modules/reitit-core/src/reitit/impl.cljc @@ -74,14 +74,15 @@ coerce (into [] (keep #(coerce % opts))))) (defn path-conflicting-routes [routes opts] - (-> (into {} - (comp (map-indexed (fn [index route] - [route (into #{} - (filter #(trie/conflicting-paths? (first route) (first %) opts)) - (subvec routes (inc index)))])) - (filter (comp seq second))) - routes) - (not-empty))) + (let [parts-and-routes (mapv (fn [[s :as r]] [(trie/split-path s opts) r]) routes)] + (-> (into {} (comp (map-indexed (fn [index [p r]] + [r (reduce + (fn [acc [p' r']] + (if (trie/conflicting-parts? p p') + (conj acc r') acc)) + #{} (subvec parts-and-routes (inc index)))])) + (filter (comp seq second))) parts-and-routes) + (not-empty)))) (defn unresolved-conflicts [path-conflicting] (-> (into {} diff --git a/modules/reitit-core/src/reitit/trie.cljc b/modules/reitit-core/src/reitit/trie.cljc index c4622b53..bbd1aded 100644 --- a/modules/reitit-core/src/reitit/trie.cljc +++ b/modules/reitit-core/src/reitit/trie.cljc @@ -132,17 +132,18 @@ (concat [(subs x i)] xs) xs))) +(defn conflicting-parts? [parts1 parts2] + (let [[[s1 & ss1] [s2 & ss2]] (-slice-start parts1 parts2)] + (cond + (= s1 s2 nil) true + (or (nil? s1) (nil? s2)) false + (or (catch-all? s1) (catch-all? s2)) true + (or (wild? s1) (wild? s2)) (recur (-slice-end s1 ss1) (-slice-end s2 ss2)) + (not= s1 s2) false + :else (recur ss1 ss2)))) + (defn conflicting-paths? [path1 path2 opts] - (loop [parts1 (split-path path1 opts) - parts2 (split-path path2 opts)] - (let [[[s1 & ss1] [s2 & ss2]] (-slice-start parts1 parts2)] - (cond - (= s1 s2 nil) true - (or (nil? s1) (nil? s2)) false - (or (catch-all? s1) (catch-all? s2)) true - (or (wild? s1) (wild? s2)) (recur (-slice-end s1 ss1) (-slice-end s2 ss2)) - (not= s1 s2) false - :else (recur ss1 ss2))))) + (conflicting-parts? (split-path path1 opts) (split-path path2 opts))) ;; ;; Creating Tries diff --git a/perf-test/clj/reitit/router_creation_perf_test.clj b/perf-test/clj/reitit/router_creation_perf_test.clj index 9349d128..4f812575 100644 --- a/perf-test/clj/reitit/router_creation_perf_test.clj +++ b/perf-test/clj/reitit/router_creation_perf_test.clj @@ -40,6 +40,7 @@ (suite "non-conflicting") ;; 104ms + ;; 11ms (reuse parts in conflict resolution) (bench! "default" (r/router hundred-routes)) ;; 7ms @@ -50,6 +51,7 @@ ;; 205ms ;; 105ms (cache path-conflicts) + ;; 13ms (reuse parts in conflict resolution) (bench! "default" (r/router routes {:conflicts nil})))) (comment