From 4163c7c36753f271b3645682ef7ac03e8800de2f Mon Sep 17 00:00:00 2001 From: Tommi Reiman Date: Sun, 5 Nov 2017 19:33:24 +0200 Subject: [PATCH 01/15] Test also composure --- .../clj/reitit/opensensors_routing_test.clj | 97 +++++++++++++++++-- 1 file changed, 90 insertions(+), 7 deletions(-) diff --git a/perf-test/clj/reitit/opensensors_routing_test.clj b/perf-test/clj/reitit/opensensors_routing_test.clj index a9f194fb..9017ac9c 100644 --- a/perf-test/clj/reitit/opensensors_routing_test.clj +++ b/perf-test/clj/reitit/opensensors_routing_test.clj @@ -5,13 +5,14 @@ [cheshire.core :as json] [clojure.string :as str] [reitit.core :as reitit] - [reitit.core :as ring] + [reitit.ring :as ring] [bidi.bidi :as bidi] [ataraxy.core :as ataraxy] [compojure.api.sweet :refer [api routes context ANY]] + [compojure.core :as compojure] [io.pedestal.http.route.definition.table :as table] [io.pedestal.http.route.map-tree :as map-tree] @@ -278,6 +279,82 @@ ["topics/" topic] [:test/route47 topic] "topics" [:test/route50]}})) +(def opensensors-compojure-routes + (compojure/routes + (compojure/context "/v1" [] + (compojure/context "/public" [] + (compojure/ANY "/topics/:topic" [] {:name :test/route4} handler) + (compojure/ANY "/users/:user-id" [] {:name :test/route16} handler) + (compojure/ANY "/orgs/:org-id" [] {:name :test/route18} handler)) + (compojure/context "/users/:user-id" [] + (compojure/ANY "/orgs/:org-id" [] {:name :test/route5} handler) + (compojure/ANY "/invitations" [] {:name :test/route7} handler) + (compojure/ANY "/topics" [] {:name :test/route9} handler) + (compojure/ANY "/bookmarks/followers" [] {:name :test/route10} handler) + (compojure/context "/devices" [] + (compojure/ANY "/" [] {:name :test/route15} handler) + #_(compojure/ANY "/bulk" [] {:name :test/route21} handler) + (compojure/ANY "/:client-id" [] {:name :test/route35} handler) + (compojure/ANY "/:client-id/reset-password" [] {:name :test/route49} handler)) + (compojure/ANY "/device-errors" [] {:name :test/route22} handler) + (compojure/ANY "/usage-stats" [] {:name :test/route24} handler) + (compojure/ANY "/claim-device/:client-id" [] {:name :test/route26} handler) + (compojure/ANY "/owned-orgs" [] {:name :test/route31} handler) + (compojure/ANY "/bookmark/:topic" [] {:name :test/route33} handler) + (compojure/ANY "/" [] {:name :test/route36} handler) + (compojure/ANY "/orgs" [] {:name :test/route52} handler) + (compojure/ANY "/api-key" [] {:name :test/route43} handler) + (compojure/ANY "/bookmarks" [] {:name :test/route56} handler)) + (compojure/ANY "/search/topics/:term" [] {:name :test/route6} handler) + (compojure/context "/orgs" [] + (compojure/ANY "/" [] {:name :test/route55} handler) + (compojure/context "/:org-id" [] + (compojure/context "/devices" [] + (compojure/ANY "/" [] {:name :test/route37} handler) + (compojure/ANY "/:device-id" [] {:name :test/route13} handler) + #_(compojure/ANY "/:batch/:type" [] {:name :test/route8} handler)) + (compojure/ANY "/usage-stats" [] {:name :test/route12} handler) + (compojure/ANY "/invitations" [] {:name :test/route19} handler) + (compojure/context "/members" [] + (compojure/ANY "/:user-id" [] {:name :test/route34} handler) + (compojure/ANY "/" [] {:name :test/route38} handler) + #_(compojure/ANY "/invitation-data/:user-id" [] {:name :test/route39} handler)) + (compojure/ANY "/errors" [] {:name :test/route17} handler) + (compojure/ANY "/" [] {:name :test/route42} handler) + (compojure/ANY "/confirm-membership/:token" [] {:name :test/route46} handler) + (compojure/ANY "/topics" [] {:name :test/route57} handler))) + (compojure/context "/messages" [] + (compojure/ANY "/user/:user-id" [] {:name :test/route14} handler) + (compojure/ANY "/device/:client-id" [] {:name :test/route30} handler) + (compojure/ANY "/topic/:topic" [] {:name :test/route48} handler)) + (compojure/context "/topics" [] + (compojure/ANY "/:topic" [] {:name :test/route32} handler) + (compojure/ANY "/" [] {:name :test/route54} handler)) + (compojure/ANY "/whoami" [] {:name :test/route41} handler) + (compojure/ANY "/login" [] {:name :test/route51} handler)) + (compojure/context "/v2" [] + (compojure/ANY "/whoami" [] {:name :test/route1} handler) + (compojure/context "/users/:user-id" [] + (compojure/ANY "/datasets" [] {:name :test/route2} handler) + (compojure/ANY "/devices" [] {:name :test/route25} handler) + (compojure/context "/topics" [] + (compojure/ANY "/bulk" [] {:name :test/route29} handler) + (compojure/ANY "/" [] {:name :test/route54} handler)) + (compojure/ANY "/" [] {:name :test/route45} handler)) + (compojure/context "/public" [] + (compojure/context "/projects/:project-id" [] + (compojure/ANY "/datasets" [] {:name :test/route3} handler) + (compojure/ANY "/" [] {:name :test/route27} handler)) + #_(compojure/ANY "/messages/dataset/bulk" [] {:name :test/route20} handler) + (compojure/ANY "/datasets/:dataset-id" [] {:name :test/route28} handler) + (compojure/ANY "/messages/dataset/:dataset-id" [] {:name :test/route53} handler)) + (compojure/ANY "/datasets/:dataset-id" [] {:name :test/route11} handler) + (compojure/ANY "/login" [] {:name :test/route23} handler) + (compojure/ANY "/orgs/:org-id/topics" [] {:name :test/route40} handler) + (compojure/ANY "/schemas" [] {:name :test/route44} handler) + (compojure/ANY "/topics/:topic" [] {:name :test/route47} handler) + (compojure/ANY "/topics" [] {:name :test/route50} handler)))) + (def opensensors-compojure-api-routes (routes (context "/v1" [] @@ -490,25 +567,31 @@ #(app {:uri % :request-method :get})) bidi-f #(bidi/match-route opensensors-bidi-routes %) ataraxy-f #(ataraxy/matches opensensors-ataraxy-routes {:uri %}) + compojure-f #(opensensors-compojure-routes {:uri % :request-method :get}) compojure-api-f #(opensensors-compojure-api-routes {:uri % :request-method :get}) pedestal-f #(pedestal/find-route opensensors-pedestal-routes {:path-info % :request-method :get})] - ;; 2538ns -> 2028ns + ;; 2538ns + ;; 2065ns (bench!! routes true "reitit" reitit-f) - ;; 2845ns -> 2299ns + ;; 2845ns + ;; 2316ns (bench!! routes true "reitit-ring" reitit-ring-f) - ;; 2737ns + ;; 2541ns (bench!! routes true "pedestal" pedestal-f) - ;; 9823ns + ;; 9462ns (bench!! routes true "compojure-api" compojure-api-f) - ;; 16716ns + ;; 11041ns + (bench!! routes true "compojure" compojure-f) + + ;; 16820ns (bench!! routes true "bidi" bidi-f) - ;; 24467ns + ;; 24134ns (bench!! routes true "ataraxy" ataraxy-f))) (comment From 16116d3e5869209bc14d4135b491a7e572bdf05a Mon Sep 17 00:00:00 2001 From: Tommi Reiman Date: Sun, 5 Nov 2017 19:33:36 +0200 Subject: [PATCH 02/15] quickbenck over rest-routes --- .../clj/reitit/opensensors_routing_test.clj | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/perf-test/clj/reitit/opensensors_routing_test.clj b/perf-test/clj/reitit/opensensors_routing_test.clj index 9017ac9c..83041fbf 100644 --- a/perf-test/clj/reitit/opensensors_routing_test.clj +++ b/perf-test/clj/reitit/opensensors_routing_test.clj @@ -73,19 +73,11 @@ total 10000 dropped (int (* total 0.45))] (mapv - #(let [times (->> (range total) - (mapv - (fn [_] - (let [now (System/nanoTime) - result (f %) - total (- (System/nanoTime) now)] - (assert result) - total))) - (sort) - (drop dropped) - (drop-last dropped)) - avg (int (/ (reduce + times) (count times)))] - [% avg]) urls))) + (fn [path] + (let [time (int (* (first (:sample-mean (cc/quick-benchmark (dotimes [_ 1000] (f path)) {}))) 1e6))] + (println path "=>" time "ns") + [path time])) + urls))) (defn bench [routes no-paths?] (let [routes (mapv (fn [[path name]] From 39bc633576cc12cafb66872c1d18b887a075094a Mon Sep 17 00:00:00 2001 From: Tommi Reiman Date: Sun, 5 Nov 2017 20:55:46 +0200 Subject: [PATCH 03/15] j.u.HashMap fails with nil --- 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 75da4dcf..f6eda250 100644 --- a/modules/reitit-core/src/reitit/impl.cljc +++ b/modules/reitit-core/src/reitit/impl.cljc @@ -161,7 +161,7 @@ :cljs [[a k v] (assoc a k v)])) (defn fast-map [m] - #?@(:clj [(java.util.HashMap. m)] + #?@(:clj [(java.util.HashMap. (or m {}))] :cljs [m])) (defn fast-get From 07861f43f919338a487a6b9a458b06b2e7eea943 Mon Sep 17 00:00:00 2001 From: Tommi Reiman Date: Sun, 5 Nov 2017 21:03:19 +0200 Subject: [PATCH 04/15] prefix-tree impl WIP MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * opensensors route match, 2.3µs => 1.3µs * static routes looked first * fixes pedestal #532 --- perf-test/clj/reitit/trie.cljc | 317 +++++++++++++++++++++++++++++++++ 1 file changed, 317 insertions(+) create mode 100644 perf-test/clj/reitit/trie.cljc diff --git a/perf-test/clj/reitit/trie.cljc b/perf-test/clj/reitit/trie.cljc new file mode 100644 index 00000000..38acec7f --- /dev/null +++ b/perf-test/clj/reitit/trie.cljc @@ -0,0 +1,317 @@ +(ns reitit.trie + (:require [clojure.walk :as walk] + [clojure.string :as str] + [criterium.core :as cc] + [reitit.impl :as impl])) + +(set! *warn-on-reflection* true) + +;; +;; Prefix-tree-router +;; + +(defn- char-key + "Return the single character child key for the string started at + index i." + [s i] + (if (< i (count s)) + (subs s i (inc i)))) + +(defprotocol Lookup + (lookup [this path params])) + +(extend-protocol Lookup + nil + (lookup [_ _ _])) + +(defrecord WildNode [segment children param wild catch] + Lookup + (lookup [this path params] + #_(println "w=>" segment "..." path) + (let [i (.indexOf ^String path "/")] + (if (pos? i) + (let [value (subs path 0 i)] + (let [childs [(impl/fast-get children (char-key path (inc i))) wild catch] + path' (subs path (inc i)) + params (assoc params param value)] + (some #(lookup % path' params) childs))) + (assoc params param path))))) + +(defrecord CatchAllNode [segment children param] + Lookup + (lookup [this path params] + (assoc params param path))) + +(defrecord StaticNode [^String segment ^Integer size children wild catch] + Lookup + (lookup [this path params] + #_(println "s=>" segment "..." path) + (if (.equals segment path) + params + (let [p (if (>= (count path) size) (subs path 0 size))] + (if (.equals segment p) + (let [childs [(impl/fast-get children (char-key path size)) wild catch] + path (subs path size)] + (some #(lookup % path params) childs))))))) + +(defn- wild? [s] + (contains? #{\: \*} (first s))) + +(defn- wild-param? + "Return true if a string segment starts with a wildcard string." + [segment] + (= \: (first segment))) + +(defn- catch-all-param? + "Return true if a string segment starts with a catch-all string." + [segment] + (= \* (first segment))) + +(defn partition-wilds + "Given a path-spec string, return a seq of strings with wildcards + and catch-alls separated into their own strings. Eats the forward + slash following a wildcard." + [path-spec] + (let [groups (partition-by wild? (str/split path-spec #"/")) + first-groups (butlast groups) + last-group (last groups)] + (flatten + (conj (mapv #(if (wild? (first %)) + % + (str (str/join "/" %) "/")) + first-groups) + (if (wild? (first last-group)) + last-group + (str/join "/" last-group)))))) + +(defn contains-wilds? + "Return true if the given path-spec contains any wildcard params or + catch-alls." + [path-spec] + (let [parts (partition-wilds path-spec)] + (or (> (count parts) 1) + (wild? (first parts))))) + +(defn- make-node + "Given a path-spec segment string and a payload object, return a new + tree node." + [segment o] + (cond + (wild-param? segment) + (map->WildNode + {:segment segment + :param (keyword (subs segment 1))}) + + (catch-all-param? segment) + (map->CatchAllNode + {:segment segment + :param (keyword (subs segment 1))}) + + :else + (map->StaticNode + {:segment segment}))) + +(defn- add-child + "Given a tree node, a single char string key and a child node, + return a new tree where this node has this child." + [node key child] + (assoc-in node [:children key] child)) + +(declare insert) + +(defn- insert-child + "Given a tree node, a single char string key, a path-spec string and + a payload object, return a tree where this object has been instered + at path-spec under this node." + [node key path-spec o] + (update-in node [:children key] insert path-spec o)) + +(defn- new-node + "Given a path-spec and a payload object, return a new tree node. If + the path-spec contains wildcards or catch-alls, will return parent + node of a tree (linked list)." + [path-spec o] + (if (contains-wilds? path-spec) + (let [parts (partition-wilds path-spec)] + (reduce (fn [child segment] + (when (catch-all-param? segment) + (throw (ex-info "catch-all may only appear at the end of a path spec" + {:patch-spec path-spec}))) + (-> (make-node segment nil) + (add-child (subs (:segment child) 0 1) child))) + (let [segment (last parts)] + (make-node segment o)) + (reverse (butlast parts)))) + (make-node path-spec o))) + +(defn- calc-lcs + "Given two strings, return the end index of the longest common + prefix string." + [s1 s2] + (loop [i 1] + (cond (or (< (count s1) i) + (< (count s2) i)) + (dec i) + + (= (subs s1 0 i) + (subs s2 0 i)) + (recur (inc i)) + + :else (dec i)))) + +(defn- split + "Given a node, a path-spec, a payload object to insert into the tree + and the lcs, split the node and return a new parent node with the + old contents of node and the new item as children. + lcs is the index of the longest common string in path-spec and the + segment of node." + [node path-spec o lcs] + (let [segment (:segment node) + common (subs path-spec 0 lcs) + parent (new-node common nil)] + (if (= common path-spec) + (-> parent + (add-child (char-key segment lcs) + (update-in node [:segment] subs lcs))) + (-> parent + (add-child (char-key segment lcs) + (update-in node [:segment] subs lcs)) + (insert-child (char-key path-spec lcs) (subs path-spec lcs) o))))) + +(defn insert + "Given a tree node, a path-spec and a payload object, return a new + tree with payload inserted." + [node path-spec o] + (let [segment (:segment node)] + (cond (nil? node) + (new-node path-spec o) + + (= segment path-spec) + node + + ;; handle case where path-spec is a wildcard param + (wild-param? path-spec) + (let [lcs (calc-lcs segment path-spec) + common (subs path-spec 0 lcs)] + (if (= common segment) + (let [path-spec (subs path-spec (inc lcs))] + (insert-child node (subs path-spec 0 1) path-spec o)) + (throw (ex-info "route conflict" + {:node node + :path-spec path-spec + :segment segment})))) + + ;; in the case where path-spec is a catch-all, node should always be nil. + ;; getting here means we have an invalid route specification + (catch-all-param? path-spec) + (throw (ex-info "route conflict" + {:node node + :path-spec path-spec + :segment segment})) + + :else + (let [lcs (calc-lcs segment path-spec)] + (cond (= lcs (count segment)) + (insert-child node (char-key path-spec lcs) (subs path-spec lcs) o) + + :else + (split node path-spec o lcs)))))) + +(defn optimize [tree] + (walk/postwalk + (fn [x] + (if (or (instance? StaticNode x) + (instance? WildNode x)) + (let [wild-child (get-in x [:children ":"]) + catch-all-child (get-in x [:children "*"])] + (cond-> x + wild-child (-> (assoc :wild wild-child) + (update :children dissoc ":")) + catch-all-child (-> (assoc :catch catch-all-child) + (update :children dissoc "*")) + (:segment x) (assoc :size (-> x :segment count)) + true (update :children impl/fast-map))) + x)) + tree)) + +;; +;; testing +;; + +(def routes + [["/v2/whoami" {:name :test/route1}] + ["/v2/users/:user-id/datasets" {:name :test/route2}] + ["/v2/public/projects/:project-id/datasets" {:name :test/route3}] + ["/v1/public/topics/:topic" {:name :test/route4}] + ["/v1/users/:user-id/orgs/:org-id" {:name :test/route5}] + ["/v1/search/topics/:term" {:name :test/route6}] + ["/v1/users/:user-id/invitations" {:name :test/route7}] + ["/v1/users/:user-id/topics" {:name :test/route9}] + ["/v1/users/:user-id/bookmarks/followers" {:name :test/route10}] + ["/v2/datasets/:dataset-id" {:name :test/route11}] + ["/v1/orgs/:org-id/usage-stats" {:name :test/route12}] + ["/v1/orgs/:org-id/devices/:client-id" {:name :test/route13}] + ["/v1/messages/user/:user-id" {:name :test/route14}] + ["/v1/users/:user-id/devices" {:name :test/route15}] + ["/v1/public/users/:user-id" {:name :test/route16}] + ["/v1/orgs/:org-id/errors" {:name :test/route17}] + ["/v1/public/orgs/:org-id" {:name :test/route18}] + ["/v1/orgs/:org-id/invitations" {:name :test/route19}] + ["/v1/users/:user-id/device-errors" {:name :test/route22}] + ["/v2/login" {:name :test/route23}] + ["/v1/users/:user-id/usage-stats" {:name :test/route24}] + ["/v2/users/:user-id/devices" {:name :test/route25}] + ["/v1/users/:user-id/claim-device/:client-id" {:name :test/route26}] + ["/v2/public/projects/:project-id" {:name :test/route27}] + ["/v2/public/datasets/:dataset-id" {:name :test/route28}] + ["/v2/users/:user-id/topics/bulk" {:name :test/route29}] + ["/v1/messages/device/:client-id" {:name :test/route30}] + ["/v1/users/:user-id/owned-orgs" {:name :test/route31}] + ["/v1/topics/:topic" {:name :test/route32}] + ["/v1/users/:user-id/bookmark/:topic" {:name :test/route33}] + ["/v1/orgs/:org-id/members/:user-id" {:name :test/route34}] + ["/v1/users/:user-id/devices/:client-id" {:name :test/route35}] + ["/v1/users/:user-id" {:name :test/route36}] + ["/v1/orgs/:org-id/devices" {:name :test/route37}] + ["/v1/orgs/:org-id/members" {:name :test/route38}] + ["/v2/orgs/:org-id/topics" {:name :test/route40}] + ["/v1/whoami" {:name :test/route41}] + ["/v1/orgs/:org-id" {:name :test/route42}] + ["/v1/users/:user-id/api-key" {:name :test/route43}] + ["/v2/schemas" {:name :test/route44}] + ["/v2/users/:user-id/topics" {:name :test/route45}] + ["/v1/orgs/:org-id/confirm-membership/:token" {:name :test/route46}] + ["/v2/topics/:topic" {:name :test/route47}] + ["/v1/messages/topic/:topic" {:name :test/route48}] + ["/v1/users/:user-id/devices/:client-id/reset-password" {:name :test/route49}] + ["/v2/topics" {:name :test/route50}] + ["/v1/login" {:name :test/route51}] + ["/v1/users/:user-id/orgs" {:name :test/route52}] + ["/v2/public/messages/dataset/:dataset-id" {:name :test/route53}] + ["/v1/topics" {:name :test/route54}] + ["/v1/orgs" {:name :test/route55}] + ["/v1/users/:user-id/bookmarks" {:name :test/route56}] + ["/v1/orgs/:org-id/topics" {:name :test/route57}]]) + +(comment + (require '[io.pedestal.http.route.prefix-tree :as p]) + (def tree-old (reduce (fn [acc [p d]] (p/insert acc p d)) nil routes)) + + ;; 2.3ms + (cc/quick-bench (dotimes [_ 1000] (p/lookup tree-old "/v1/orgs/1/topics")))) + +(comment + (def tree-new (optimize (reduce (fn [acc [p d]] (insert acc p d)) nil routes))) + + ;; 3.1ms + ;; 2.5ms (string equals) + ;; 2.5ms (protocol) + ;; 2.3ms (nil childs) + ;; 2.0ms (rando impros) + ;; 1.9ms (wild & catch shortcuts) + ;; 1.5ms (inline child fetching) + ;; 1.5ms (WildNode also backtracks) + ;; 1.4ms (precalculate segment-size) + ;; 1.3ms (fast-map) + ;; 1.3ms (dissoc wild & catch-all from children) + (cc/quick-bench (dotimes [_ 1000] (lookup tree-new "/v1/orgs/1/topics" {})))) From eeea39ca83086f4d0ac05c551365403bb1ef531c Mon Sep 17 00:00:00 2001 From: Tommi Reiman Date: Tue, 7 Nov 2017 20:30:26 +0200 Subject: [PATCH 05/15] Re-implement using reified protocols * tree can be optimzied on the fly! --- perf-test/clj/reitit/trie.cljc | 159 +++++++++++++++++---------------- 1 file changed, 80 insertions(+), 79 deletions(-) diff --git a/perf-test/clj/reitit/trie.cljc b/perf-test/clj/reitit/trie.cljc index 38acec7f..523f22ea 100644 --- a/perf-test/clj/reitit/trie.cljc +++ b/perf-test/clj/reitit/trie.cljc @@ -10,6 +10,8 @@ ;; Prefix-tree-router ;; +(declare insert) + (defn- char-key "Return the single character child key for the string started at index i." @@ -17,42 +19,79 @@ (if (< i (count s)) (subs s i (inc i)))) -(defprotocol Lookup - (lookup [this path params])) +(defn- maybe-wild-node [children] + (get children ":")) -(extend-protocol Lookup +(defn- maybe-catch-all-node [children] + (get children "*")) + +(defprotocol Node + (lookup [this path params]) + (get-segment [this]) + (update-segment [this subs lcs]) + (add-child [this key child]) + (insert-child [this key path-spec o])) + +(extend-protocol Node nil - (lookup [_ _ _])) + (lookup [_ _ _]) + (get-segment [this])) -(defrecord WildNode [segment children param wild catch] - Lookup - (lookup [this path params] - #_(println "w=>" segment "..." path) - (let [i (.indexOf ^String path "/")] - (if (pos? i) - (let [value (subs path 0 i)] - (let [childs [(impl/fast-get children (char-key path (inc i))) wild catch] - path' (subs path (inc i)) - params (assoc params param value)] - (some #(lookup % path' params) childs))) - (assoc params param path))))) +(defn wild-node [segment param children] + (let [?wild (maybe-wild-node children) + ?catch (maybe-catch-all-node children) + children' (impl/fast-map children)] + (reify + Node + (lookup [this path params] + #_(println "w=>" segment "..." path) + (let [i (.indexOf ^String path "/")] + (if (pos? i) + (let [value (subs path 0 i)] + (let [childs [(impl/fast-get children' (char-key path (inc i))) ?wild ?catch] + path' (subs path (inc i)) + params (assoc params param value)] + (some #(lookup % path' params) childs))) + (assoc params param path)))) + (get-segment [this] + segment) + (add-child [this key child] + (wild-node segment param (assoc children key child))) + (insert-child [this key path-spec o] + (wild-node segment param (update children key insert path-spec o)))))) -(defrecord CatchAllNode [segment children param] - Lookup - (lookup [this path params] - (assoc params param path))) +(defn catch-all-node [segment children param] + (reify + Node + (lookup [this path params] + (assoc params param path)) + (get-segment [this] + segment))) -(defrecord StaticNode [^String segment ^Integer size children wild catch] - Lookup - (lookup [this path params] - #_(println "s=>" segment "..." path) - (if (.equals segment path) - params - (let [p (if (>= (count path) size) (subs path 0 size))] - (if (.equals segment p) - (let [childs [(impl/fast-get children (char-key path size)) wild catch] - path (subs path size)] - (some #(lookup % path params) childs))))))) +(defn static-node [^String segment children] + (let [size (count segment) + ?wild (maybe-wild-node children) + ?catch (maybe-catch-all-node children) + children' (impl/fast-map children)] + (reify + Node + (lookup [this path params] + #_(println "s=>" segment "..." path) + (if (.equals segment path) + params + (let [p (if (>= (count path) size) (subs path 0 size))] + (if (.equals segment p) + (let [childs [(impl/fast-get children' (char-key path size)) ?wild ?catch] + path (subs path size)] + (some #(lookup % path params) childs)))))) + (get-segment [this] + segment) + (update-segment [this subs lcs] + (static-node (subs segment lcs) children)) + (add-child [this key child] + (static-node segment (assoc children key child))) + (insert-child [this key path-spec o] + (static-node segment (update children key insert path-spec o)))))) (defn- wild? [s] (contains? #{\: \*} (first s))) @@ -98,33 +137,13 @@ [segment o] (cond (wild-param? segment) - (map->WildNode - {:segment segment - :param (keyword (subs segment 1))}) + (wild-node segment (keyword (subs segment 1)) nil) (catch-all-param? segment) - (map->CatchAllNode - {:segment segment - :param (keyword (subs segment 1))}) + (catch-all-node segment (keyword (subs segment 1)) nil) :else - (map->StaticNode - {:segment segment}))) - -(defn- add-child - "Given a tree node, a single char string key and a child node, - return a new tree where this node has this child." - [node key child] - (assoc-in node [:children key] child)) - -(declare insert) - -(defn- insert-child - "Given a tree node, a single char string key, a path-spec string and - a payload object, return a tree where this object has been instered - at path-spec under this node." - [node key path-spec o] - (update-in node [:children key] insert path-spec o)) + (static-node segment nil))) (defn- new-node "Given a path-spec and a payload object, return a new tree node. If @@ -138,7 +157,7 @@ (throw (ex-info "catch-all may only appear at the end of a path spec" {:patch-spec path-spec}))) (-> (make-node segment nil) - (add-child (subs (:segment child) 0 1) child))) + (add-child (subs (get-segment child) 0 1) child))) (let [segment (last parts)] (make-node segment o)) (reverse (butlast parts)))) @@ -166,23 +185,21 @@ lcs is the index of the longest common string in path-spec and the segment of node." [node path-spec o lcs] - (let [segment (:segment node) + (let [segment (get-segment node) common (subs path-spec 0 lcs) parent (new-node common nil)] (if (= common path-spec) (-> parent - (add-child (char-key segment lcs) - (update-in node [:segment] subs lcs))) + (add-child (char-key segment lcs) (update-segment node subs lcs))) (-> parent - (add-child (char-key segment lcs) - (update-in node [:segment] subs lcs)) + (add-child (char-key segment lcs) (update-segment node subs lcs)) (insert-child (char-key path-spec lcs) (subs path-spec lcs) o))))) (defn insert "Given a tree node, a path-spec and a payload object, return a new tree with payload inserted." [node path-spec o] - (let [segment (:segment node)] + (let [segment (get-segment node)] (cond (nil? node) (new-node path-spec o) @@ -217,23 +234,6 @@ :else (split node path-spec o lcs)))))) -(defn optimize [tree] - (walk/postwalk - (fn [x] - (if (or (instance? StaticNode x) - (instance? WildNode x)) - (let [wild-child (get-in x [:children ":"]) - catch-all-child (get-in x [:children "*"])] - (cond-> x - wild-child (-> (assoc :wild wild-child) - (update :children dissoc ":")) - catch-all-child (-> (assoc :catch catch-all-child) - (update :children dissoc "*")) - (:segment x) (assoc :size (-> x :segment count)) - true (update :children impl/fast-map))) - x)) - tree)) - ;; ;; testing ;; @@ -301,7 +301,7 @@ (cc/quick-bench (dotimes [_ 1000] (p/lookup tree-old "/v1/orgs/1/topics")))) (comment - (def tree-new (optimize (reduce (fn [acc [p d]] (insert acc p d)) nil routes))) + (def tree-new (reduce (fn [acc [p d]] (insert acc p d)) nil routes)) ;; 3.1ms ;; 2.5ms (string equals) @@ -314,4 +314,5 @@ ;; 1.4ms (precalculate segment-size) ;; 1.3ms (fast-map) ;; 1.3ms (dissoc wild & catch-all from children) + ;; 1.3ms (reified protocols) (cc/quick-bench (dotimes [_ 1000] (lookup tree-new "/v1/orgs/1/topics" {})))) From e12bfeabf154502b29564599c693f1f8619c76a0 Mon Sep 17 00:00:00 2001 From: Tommi Reiman Date: Tue, 7 Nov 2017 20:36:26 +0200 Subject: [PATCH 06/15] Flattened matching, 1.2ms -> 0.8ms!!! --- perf-test/clj/reitit/trie.cljc | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/perf-test/clj/reitit/trie.cljc b/perf-test/clj/reitit/trie.cljc index 523f22ea..67530998 100644 --- a/perf-test/clj/reitit/trie.cljc +++ b/perf-test/clj/reitit/trie.cljc @@ -44,14 +44,15 @@ (reify Node (lookup [this path params] - #_(println "w=>" segment "..." path) (let [i (.indexOf ^String path "/")] (if (pos? i) (let [value (subs path 0 i)] - (let [childs [(impl/fast-get children' (char-key path (inc i))) ?wild ?catch] + (let [child (impl/fast-get children' (char-key path (inc i))) path' (subs path (inc i)) params (assoc params param value)] - (some #(lookup % path' params) childs))) + (or (lookup child path' params) + (lookup ?wild path' params) + (lookup ?catch path' params)))) (assoc params param path)))) (get-segment [this] segment) @@ -76,14 +77,15 @@ (reify Node (lookup [this path params] - #_(println "s=>" segment "..." path) (if (.equals segment path) params (let [p (if (>= (count path) size) (subs path 0 size))] (if (.equals segment p) - (let [childs [(impl/fast-get children' (char-key path size)) ?wild ?catch] + (let [child (impl/fast-get children' (char-key path size)) path (subs path size)] - (some #(lookup % path params) childs)))))) + (or (lookup child path params) + (lookup ?wild path params) + (lookup ?catch path params))))))) (get-segment [this] segment) (update-segment [this subs lcs] @@ -315,4 +317,5 @@ ;; 1.3ms (fast-map) ;; 1.3ms (dissoc wild & catch-all from children) ;; 1.3ms (reified protocols) + ;; 0.8ms (flattened matching) (cc/quick-bench (dotimes [_ 1000] (lookup tree-new "/v1/orgs/1/topics" {})))) From 32a1be14662a33b0258543a39c2f30e0162fe013 Mon Sep 17 00:00:00 2001 From: Tommi Reiman Date: Tue, 7 Nov 2017 21:02:28 +0200 Subject: [PATCH 07/15] Return also route-data --- perf-test/clj/reitit/trie.cljc | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/perf-test/clj/reitit/trie.cljc b/perf-test/clj/reitit/trie.cljc index 67530998..6416a735 100644 --- a/perf-test/clj/reitit/trie.cljc +++ b/perf-test/clj/reitit/trie.cljc @@ -37,7 +37,9 @@ (lookup [_ _ _]) (get-segment [this])) -(defn wild-node [segment param children] +(defrecord Match [data params]) + +(defn wild-node [segment param children data] (let [?wild (maybe-wild-node children) ?catch (maybe-catch-all-node children) children' (impl/fast-map children)] @@ -53,23 +55,23 @@ (or (lookup child path' params) (lookup ?wild path' params) (lookup ?catch path' params)))) - (assoc params param path)))) + (->Match data (assoc params param path))))) (get-segment [this] segment) (add-child [this key child] - (wild-node segment param (assoc children key child))) + (wild-node segment param (assoc children key child) data)) (insert-child [this key path-spec o] - (wild-node segment param (update children key insert path-spec o)))))) + (wild-node segment param (update children key insert path-spec o) data))))) -(defn catch-all-node [segment children param] +(defn catch-all-node [segment children param data] (reify Node (lookup [this path params] - (assoc params param path)) + (->Match data (assoc params param path))) (get-segment [this] segment))) -(defn static-node [^String segment children] +(defn static-node [^String segment children data] (let [size (count segment) ?wild (maybe-wild-node children) ?catch (maybe-catch-all-node children) @@ -78,7 +80,7 @@ Node (lookup [this path params] (if (.equals segment path) - params + (->Match data params) (let [p (if (>= (count path) size) (subs path 0 size))] (if (.equals segment p) (let [child (impl/fast-get children' (char-key path size)) @@ -89,11 +91,11 @@ (get-segment [this] segment) (update-segment [this subs lcs] - (static-node (subs segment lcs) children)) + (static-node (subs segment lcs) children data)) (add-child [this key child] - (static-node segment (assoc children key child))) + (static-node segment (assoc children key child) data)) (insert-child [this key path-spec o] - (static-node segment (update children key insert path-spec o)))))) + (static-node segment (update children key insert path-spec o) data))))) (defn- wild? [s] (contains? #{\: \*} (first s))) @@ -139,13 +141,13 @@ [segment o] (cond (wild-param? segment) - (wild-node segment (keyword (subs segment 1)) nil) + (wild-node segment (keyword (subs segment 1)) nil o) (catch-all-param? segment) - (catch-all-node segment (keyword (subs segment 1)) nil) + (catch-all-node segment (keyword (subs segment 1)) nil o) :else - (static-node segment nil))) + (static-node segment nil o))) (defn- new-node "Given a path-spec and a payload object, return a new tree node. If @@ -318,4 +320,5 @@ ;; 1.3ms (dissoc wild & catch-all from children) ;; 1.3ms (reified protocols) ;; 0.8ms (flattened matching) + ;; 0.8ms (return route-data) (cc/quick-bench (dotimes [_ 1000] (lookup tree-new "/v1/orgs/1/topics" {})))) From c378d0b5afd66b4b41f220e99929cb0990efa7a2 Mon Sep 17 00:00:00 2001 From: Tommi Reiman Date: Tue, 7 Nov 2017 22:55:05 +0200 Subject: [PATCH 08/15] fix payloads --- perf-test/clj/reitit/trie.cljc | 42 +++++++++++++++++++--------------- 1 file changed, 24 insertions(+), 18 deletions(-) diff --git a/perf-test/clj/reitit/trie.cljc b/perf-test/clj/reitit/trie.cljc index 6416a735..2fd5e49d 100644 --- a/perf-test/clj/reitit/trie.cljc +++ b/perf-test/clj/reitit/trie.cljc @@ -29,6 +29,7 @@ (lookup [this path params]) (get-segment [this]) (update-segment [this subs lcs]) + (set-data [this data]) (add-child [this key child]) (insert-child [this key path-spec o])) @@ -58,6 +59,8 @@ (->Match data (assoc params param path))))) (get-segment [this] segment) + (set-data [this data] + (wild-node segment param children data)) (add-child [this key child] (wild-node segment param (assoc children key child) data)) (insert-child [this key path-spec o] @@ -92,10 +95,12 @@ segment) (update-segment [this subs lcs] (static-node (subs segment lcs) children data)) + (set-data [this data] + (static-node segment children data)) (add-child [this key child] (static-node segment (assoc children key child) data)) - (insert-child [this key path-spec o] - (static-node segment (update children key insert path-spec o) data))))) + (insert-child [this key path-spec child-data] + (static-node segment (update children key insert path-spec child-data) data))))) (defn- wild? [s] (contains? #{\: \*} (first s))) @@ -138,22 +143,22 @@ (defn- make-node "Given a path-spec segment string and a payload object, return a new tree node." - [segment o] + [segment data] (cond (wild-param? segment) - (wild-node segment (keyword (subs segment 1)) nil o) + (wild-node segment (keyword (subs segment 1)) nil data) (catch-all-param? segment) - (catch-all-node segment (keyword (subs segment 1)) nil o) + (catch-all-node segment (keyword (subs segment 1)) nil data) :else - (static-node segment nil o))) + (static-node segment nil data))) (defn- new-node "Given a path-spec and a payload object, return a new tree node. If the path-spec contains wildcards or catch-alls, will return parent node of a tree (linked list)." - [path-spec o] + [path-spec data] (if (contains-wilds? path-spec) (let [parts (partition-wilds path-spec)] (reduce (fn [child segment] @@ -163,9 +168,9 @@ (-> (make-node segment nil) (add-child (subs (get-segment child) 0 1) child))) (let [segment (last parts)] - (make-node segment o)) + (make-node segment data)) (reverse (butlast parts)))) - (make-node path-spec o))) + (make-node path-spec data))) (defn- calc-lcs "Given two strings, return the end index of the longest common @@ -188,27 +193,27 @@ old contents of node and the new item as children. lcs is the index of the longest common string in path-spec and the segment of node." - [node path-spec o lcs] + [node path-spec data lcs] (let [segment (get-segment node) common (subs path-spec 0 lcs) parent (new-node common nil)] (if (= common path-spec) - (-> parent + (-> (set-data parent data) (add-child (char-key segment lcs) (update-segment node subs lcs))) (-> parent (add-child (char-key segment lcs) (update-segment node subs lcs)) - (insert-child (char-key path-spec lcs) (subs path-spec lcs) o))))) + (insert-child (char-key path-spec lcs) (subs path-spec lcs) data))))) (defn insert "Given a tree node, a path-spec and a payload object, return a new tree with payload inserted." - [node path-spec o] + [node path-spec data] (let [segment (get-segment node)] (cond (nil? node) - (new-node path-spec o) + (new-node path-spec data) (= segment path-spec) - node + (set-data node data) ;; handle case where path-spec is a wildcard param (wild-param? path-spec) @@ -216,7 +221,7 @@ common (subs path-spec 0 lcs)] (if (= common segment) (let [path-spec (subs path-spec (inc lcs))] - (insert-child node (subs path-spec 0 1) path-spec o)) + (insert-child node (subs path-spec 0 1) path-spec data)) (throw (ex-info "route conflict" {:node node :path-spec path-spec @@ -233,10 +238,10 @@ :else (let [lcs (calc-lcs segment path-spec)] (cond (= lcs (count segment)) - (insert-child node (char-key path-spec lcs) (subs path-spec lcs) o) + (insert-child node (char-key path-spec lcs) (subs path-spec lcs) data) :else - (split node path-spec o lcs)))))) + (split node path-spec data lcs)))))) ;; ;; testing @@ -321,4 +326,5 @@ ;; 1.3ms (reified protocols) ;; 0.8ms (flattened matching) ;; 0.8ms (return route-data) + ;; 0.8ms (fix payloads) (cc/quick-bench (dotimes [_ 1000] (lookup tree-new "/v1/orgs/1/topics" {})))) From 0d63aa1d431ccfe6f5565af1dcef20696ba7298a Mon Sep 17 00:00:00 2001 From: Tommi Reiman Date: Tue, 7 Nov 2017 22:56:58 +0200 Subject: [PATCH 09/15] Cleanup arguments --- perf-test/clj/reitit/trie.cljc | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/perf-test/clj/reitit/trie.cljc b/perf-test/clj/reitit/trie.cljc index 2fd5e49d..6cd0a5e8 100644 --- a/perf-test/clj/reitit/trie.cljc +++ b/perf-test/clj/reitit/trie.cljc @@ -31,12 +31,12 @@ (update-segment [this subs lcs]) (set-data [this data]) (add-child [this key child]) - (insert-child [this key path-spec o])) + (insert-child [this key path-spec data])) (extend-protocol Node nil (lookup [_ _ _]) - (get-segment [this])) + (get-segment [_])) (defrecord Match [data params]) @@ -46,7 +46,7 @@ children' (impl/fast-map children)] (reify Node - (lookup [this path params] + (lookup [_ path params] (let [i (.indexOf ^String path "/")] (if (pos? i) (let [value (subs path 0 i)] @@ -57,21 +57,21 @@ (lookup ?wild path' params) (lookup ?catch path' params)))) (->Match data (assoc params param path))))) - (get-segment [this] + (get-segment [_] segment) - (set-data [this data] + (set-data [_ data] (wild-node segment param children data)) - (add-child [this key child] + (add-child [_ key child] (wild-node segment param (assoc children key child) data)) - (insert-child [this key path-spec o] - (wild-node segment param (update children key insert path-spec o) data))))) + (insert-child [_ key path-spec child-data] + (wild-node segment param (update children key insert path-spec child-data) data))))) (defn catch-all-node [segment children param data] (reify Node - (lookup [this path params] + (lookup [_ path params] (->Match data (assoc params param path))) - (get-segment [this] + (get-segment [_] segment))) (defn static-node [^String segment children data] @@ -81,7 +81,7 @@ children' (impl/fast-map children)] (reify Node - (lookup [this path params] + (lookup [_ path params] (if (.equals segment path) (->Match data params) (let [p (if (>= (count path) size) (subs path 0 size))] @@ -91,15 +91,15 @@ (or (lookup child path params) (lookup ?wild path params) (lookup ?catch path params))))))) - (get-segment [this] + (get-segment [_] segment) - (update-segment [this subs lcs] + (update-segment [_ subs lcs] (static-node (subs segment lcs) children data)) - (set-data [this data] + (set-data [_ data] (static-node segment children data)) - (add-child [this key child] + (add-child [_ key child] (static-node segment (assoc children key child) data)) - (insert-child [this key path-spec child-data] + (insert-child [_ key path-spec child-data] (static-node segment (update children key insert path-spec child-data) data))))) (defn- wild? [s] From 5d7786936c95997f3f491e00155298e084921cf3 Mon Sep 17 00:00:00 2001 From: Tommi Reiman Date: Wed, 8 Nov 2017 09:12:40 +0000 Subject: [PATCH 10/15] prefix-tree-router! --- modules/reitit-core/src/reitit/core.cljc | 51 +++++++++++-- modules/reitit-core/src/reitit/impl.cljc | 14 +++- .../reitit-core/src}/reitit/trie.cljc | 76 ++++--------------- .../clj/reitit/opensensors_routing_test.clj | 2 + perf-test/clj/reitit/perf_test.clj | 1 + test/cljc/reitit/core_test.cljc | 36 ++++++++- 6 files changed, 111 insertions(+), 69 deletions(-) rename {perf-test/clj => modules/reitit-core/src}/reitit/trie.cljc (83%) diff --git a/modules/reitit-core/src/reitit/core.cljc b/modules/reitit-core/src/reitit/core.cljc index 7efe387f..cec9c33d 100644 --- a/modules/reitit-core/src/reitit/core.cljc +++ b/modules/reitit-core/src/reitit/core.cljc @@ -1,6 +1,7 @@ (ns reitit.core (:require [meta-merge.core :refer [meta-merge]] [clojure.string :as str] + [reitit.trie :as trie] [reitit.impl :as impl #?@(:cljs [:refer [Route]])]) #?(:clj (:import (reitit.impl Route)))) @@ -216,6 +217,46 @@ (if-let [match (impl/fast-get lookup name)] (match params))))))) +(defn prefix-tree-router + "Creates a prefix-tree router from resolved routes and optional + expanded options. See [[router]] for available options" + ([routes] + (prefix-tree-router routes {})) + ([routes opts] + (let [compiled (compile-routes routes opts) + names (find-names routes opts) + [node lookup] (reduce + (fn [[node lookup] [p {:keys [name] :as meta} result]] + (let [{:keys [params] :as route} (impl/create [p meta result]) + f #(if-let [path (impl/path-for route %)] + (->Match p meta result % path) + (->PartialMatch p meta result % params))] + [(trie/insert node p (->Match p meta result nil nil)) + (if name (assoc lookup name f) lookup)])) + [nil {}] compiled) + lookup (impl/fast-map lookup)] + (reify + Router + (router-name [_] + :prefix-tree-router) + (routes [_] + compiled) + (options [_] + opts) + (route-names [_] + names) + (match-by-path [_ path] + (if-let [match (trie/lookup node path {})] + (-> (:data match) + (assoc :params (:params match)) + (assoc :path path)))) + (match-by-name [_ name] + (if-let [match (impl/fast-get lookup name)] + (match nil))) + (match-by-name [_ name params] + (if-let [match (impl/fast-get lookup name)] + (match params))))))) + (defn single-static-path-router "Creates a fast router of 1 static route(s) and optional expanded options. See [[router]] for available options" @@ -252,16 +293,16 @@ (defn mixed-router "Creates two routers: [[lookup-router]] or [[single-static-path-router]] for - static routes and [[linear-router]] for wildcard routes. All + static routes and [[prefix-tree-router]] for wildcard routes. All routes should be non-conflicting. Takes resolved routes and optional expanded options. See [[router]] for options." ([routes] (mixed-router routes {})) ([routes opts] - (let [{linear true, lookup false} (group-by impl/wild-route? routes) + (let [{wild true, lookup false} (group-by impl/wild-route? routes) compiled (compile-routes routes opts) ->static-router (if (= 1 (count lookup)) single-static-path-router lookup-router) - wildcard-router (linear-router linear opts) + wildcard-router (prefix-tree-router wild opts) static-router (->static-router lookup opts) names (find-names routes opts)] (reify Router @@ -310,9 +351,9 @@ router router (and (= 1 (count routes)) (not wilds?)) single-static-path-router (not wilds?) lookup-router - all-wilds? linear-router + all-wilds? prefix-tree-router (not conflicting) mixed-router - :else linear-router)] + :else prefix-tree-router)] (when-let [conflicts (:conflicts opts)] (when conflicting (conflicts conflicting))) diff --git a/modules/reitit-core/src/reitit/impl.cljc b/modules/reitit-core/src/reitit/impl.cljc index f6eda250..5694336f 100644 --- a/modules/reitit-core/src/reitit/impl.cljc +++ b/modules/reitit-core/src/reitit/impl.cljc @@ -69,10 +69,20 @@ ;; (c) https://github.com/pedestal/pedestal/blob/master/route/src/io/pedestal/http/route/prefix_tree.clj ;; -(defn- wild? [s] +(defn wild? [s] (contains? #{\: \*} (first s))) -(defn- partition-wilds +(defn wild-param? + "Return true if a string segment starts with a wildcard string." + [segment] + (= \: (first segment))) + +(defn catch-all-param? + "Return true if a string segment starts with a catch-all string." + [segment] + (= \* (first segment))) + +(defn partition-wilds "Given a path-spec string, return a seq of strings with wildcards and catch-alls separated into their own strings. Eats the forward slash following a wildcard." diff --git a/perf-test/clj/reitit/trie.cljc b/modules/reitit-core/src/reitit/trie.cljc similarity index 83% rename from perf-test/clj/reitit/trie.cljc rename to modules/reitit-core/src/reitit/trie.cljc index 6cd0a5e8..66028ae9 100644 --- a/perf-test/clj/reitit/trie.cljc +++ b/modules/reitit-core/src/reitit/trie.cljc @@ -1,21 +1,13 @@ (ns reitit.trie - (:require [clojure.walk :as walk] - [clojure.string :as str] - [criterium.core :as cc] - [reitit.impl :as impl])) - -(set! *warn-on-reflection* true) + (:require [reitit.impl :as impl])) ;; -;; Prefix-tree-router +;; original https://github.com/pedestal/pedestal/blob/master/route/src/io/pedestal/http/route/prefix_tree.clj ;; (declare insert) -(defn- char-key - "Return the single character child key for the string started at - index i." - [s i] +(defn- char-key [s i] (if (< i (count s)) (subs s i (inc i)))) @@ -40,7 +32,7 @@ (defrecord Match [data params]) -(defn wild-node [segment param children data] +(defn- wild-node [segment param children data] (let [?wild (maybe-wild-node children) ?catch (maybe-catch-all-node children) children' (impl/fast-map children)] @@ -66,7 +58,7 @@ (insert-child [_ key path-spec child-data] (wild-node segment param (update children key insert path-spec child-data) data))))) -(defn catch-all-node [segment children param data] +(defn- catch-all-node [segment children param data] (reify Node (lookup [_ path params] @@ -74,7 +66,7 @@ (get-segment [_] segment))) -(defn static-node [^String segment children data] +(defn- static-node [^String segment children data] (let [size (count segment) ?wild (maybe-wild-node children) ?catch (maybe-catch-all-node children) @@ -82,10 +74,10 @@ (reify Node (lookup [_ path params] - (if (.equals segment path) + (if (#?(:clj .equals, :cljs =) segment path) (->Match data params) (let [p (if (>= (count path) size) (subs path 0 size))] - (if (.equals segment p) + (if (#?(:clj .equals, :cljs =) segment p) (let [child (impl/fast-get children' (char-key path size)) path (subs path size)] (or (lookup child path params) @@ -102,53 +94,15 @@ (insert-child [_ key path-spec child-data] (static-node segment (update children key insert path-spec child-data) data))))) -(defn- wild? [s] - (contains? #{\: \*} (first s))) - -(defn- wild-param? - "Return true if a string segment starts with a wildcard string." - [segment] - (= \: (first segment))) - -(defn- catch-all-param? - "Return true if a string segment starts with a catch-all string." - [segment] - (= \* (first segment))) - -(defn partition-wilds - "Given a path-spec string, return a seq of strings with wildcards - and catch-alls separated into their own strings. Eats the forward - slash following a wildcard." - [path-spec] - (let [groups (partition-by wild? (str/split path-spec #"/")) - first-groups (butlast groups) - last-group (last groups)] - (flatten - (conj (mapv #(if (wild? (first %)) - % - (str (str/join "/" %) "/")) - first-groups) - (if (wild? (first last-group)) - last-group - (str/join "/" last-group)))))) - -(defn contains-wilds? - "Return true if the given path-spec contains any wildcard params or - catch-alls." - [path-spec] - (let [parts (partition-wilds path-spec)] - (or (> (count parts) 1) - (wild? (first parts))))) - (defn- make-node "Given a path-spec segment string and a payload object, return a new tree node." [segment data] (cond - (wild-param? segment) + (impl/wild-param? segment) (wild-node segment (keyword (subs segment 1)) nil data) - (catch-all-param? segment) + (impl/catch-all-param? segment) (catch-all-node segment (keyword (subs segment 1)) nil data) :else @@ -159,10 +113,10 @@ the path-spec contains wildcards or catch-alls, will return parent node of a tree (linked list)." [path-spec data] - (if (contains-wilds? path-spec) - (let [parts (partition-wilds path-spec)] + (if (impl/contains-wilds? path-spec) + (let [parts (impl/partition-wilds path-spec)] (reduce (fn [child segment] - (when (catch-all-param? segment) + (when (impl/catch-all-param? segment) (throw (ex-info "catch-all may only appear at the end of a path spec" {:patch-spec path-spec}))) (-> (make-node segment nil) @@ -216,7 +170,7 @@ (set-data node data) ;; handle case where path-spec is a wildcard param - (wild-param? path-spec) + (impl/wild-param? path-spec) (let [lcs (calc-lcs segment path-spec) common (subs path-spec 0 lcs)] (if (= common segment) @@ -229,7 +183,7 @@ ;; in the case where path-spec is a catch-all, node should always be nil. ;; getting here means we have an invalid route specification - (catch-all-param? path-spec) + (impl/catch-all-param? path-spec) (throw (ex-info "route conflict" {:node node :path-spec path-spec diff --git a/perf-test/clj/reitit/opensensors_routing_test.clj b/perf-test/clj/reitit/opensensors_routing_test.clj index 83041fbf..cd3588dc 100644 --- a/perf-test/clj/reitit/opensensors_routing_test.clj +++ b/perf-test/clj/reitit/opensensors_routing_test.clj @@ -565,10 +565,12 @@ ;; 2538ns ;; 2065ns + ;; 680ns (prefix-tree-router) (bench!! routes true "reitit" reitit-f) ;; 2845ns ;; 2316ns + ;; 947ns (prefix-tree-router) (bench!! routes true "reitit-ring" reitit-ring-f) ;; 2541ns diff --git a/perf-test/clj/reitit/perf_test.clj b/perf-test/clj/reitit/perf_test.clj index 61b6ae31..dfefd636 100644 --- a/perf-test/clj/reitit/perf_test.clj +++ b/perf-test/clj/reitit/perf_test.clj @@ -148,6 +148,7 @@ (call)))) ;; 710 µs (3-18x) + ;; 540 µs (4-23x) -23% prefix-tree-router (title "reitit") (let [call #(reitit/match-by-path reitit-routes "/workspace/1/1")] (assert (call)) diff --git a/test/cljc/reitit/core_test.cljc b/test/cljc/reitit/core_test.cljc index 669b988c..dbee0bac 100644 --- a/test/cljc/reitit/core_test.cljc +++ b/test/cljc/reitit/core_test.cljc @@ -7,8 +7,42 @@ (deftest reitit-test - (testing "linear-router" + (testing "prefix-tree-router" (let [router (r/router ["/api" ["/ipa" ["/:size" ::beer]]])] + (is (= :prefix-tree-router (r/router-name router))) + (is (= [["/api/ipa/:size" {:name ::beer} nil]] + (r/routes router))) + (is (= true (map? (r/options router)))) + (is (= (r/map->Match + {:template "/api/ipa/:size" + :meta {:name ::beer} + :path "/api/ipa/large" + :params {:size "large"}}) + (r/match-by-path router "/api/ipa/large"))) + (is (= (r/map->Match + {:template "/api/ipa/:size" + :meta {:name ::beer} + :path "/api/ipa/large" + :params {:size "large"}}) + (r/match-by-name router ::beer {:size "large"}))) + (is (= nil (r/match-by-name router "ILLEGAL"))) + (is (= [::beer] (r/route-names router))) + + (testing "name-based routing with missing parameters" + (is (= (r/map->PartialMatch + {:template "/api/ipa/:size" + :meta {:name ::beer} + :required #{:size} + :params nil}) + (r/match-by-name router ::beer))) + (is (= true (r/partial-match? (r/match-by-name router ::beer)))) + (is (thrown-with-msg? + ExceptionInfo + #"^missing path-params for route /api/ipa/:size -> \#\{:size\}$" + (r/match-by-name! router ::beer)))))) + + (testing "linear-router" + (let [router (r/router ["/api" ["/ipa" ["/:size" ::beer]]] {:router r/linear-router})] (is (= :linear-router (r/router-name router))) (is (= [["/api/ipa/:size" {:name ::beer} nil]] (r/routes router))) From de993abf6274de5a68214a0bd56129b719934d03 Mon Sep 17 00:00:00 2001 From: Tommi Reiman Date: Wed, 8 Nov 2017 09:18:52 +0000 Subject: [PATCH 11/15] Move tests --- modules/reitit-core/src/reitit/trie.cljc | 86 -------------- .../clj/reitit/prefix_tree_perf_test.clj | 105 ++++++++++++++++++ 2 files changed, 105 insertions(+), 86 deletions(-) create mode 100644 perf-test/clj/reitit/prefix_tree_perf_test.clj diff --git a/modules/reitit-core/src/reitit/trie.cljc b/modules/reitit-core/src/reitit/trie.cljc index 66028ae9..5d47eb7a 100644 --- a/modules/reitit-core/src/reitit/trie.cljc +++ b/modules/reitit-core/src/reitit/trie.cljc @@ -196,89 +196,3 @@ :else (split node path-spec data lcs)))))) - -;; -;; testing -;; - -(def routes - [["/v2/whoami" {:name :test/route1}] - ["/v2/users/:user-id/datasets" {:name :test/route2}] - ["/v2/public/projects/:project-id/datasets" {:name :test/route3}] - ["/v1/public/topics/:topic" {:name :test/route4}] - ["/v1/users/:user-id/orgs/:org-id" {:name :test/route5}] - ["/v1/search/topics/:term" {:name :test/route6}] - ["/v1/users/:user-id/invitations" {:name :test/route7}] - ["/v1/users/:user-id/topics" {:name :test/route9}] - ["/v1/users/:user-id/bookmarks/followers" {:name :test/route10}] - ["/v2/datasets/:dataset-id" {:name :test/route11}] - ["/v1/orgs/:org-id/usage-stats" {:name :test/route12}] - ["/v1/orgs/:org-id/devices/:client-id" {:name :test/route13}] - ["/v1/messages/user/:user-id" {:name :test/route14}] - ["/v1/users/:user-id/devices" {:name :test/route15}] - ["/v1/public/users/:user-id" {:name :test/route16}] - ["/v1/orgs/:org-id/errors" {:name :test/route17}] - ["/v1/public/orgs/:org-id" {:name :test/route18}] - ["/v1/orgs/:org-id/invitations" {:name :test/route19}] - ["/v1/users/:user-id/device-errors" {:name :test/route22}] - ["/v2/login" {:name :test/route23}] - ["/v1/users/:user-id/usage-stats" {:name :test/route24}] - ["/v2/users/:user-id/devices" {:name :test/route25}] - ["/v1/users/:user-id/claim-device/:client-id" {:name :test/route26}] - ["/v2/public/projects/:project-id" {:name :test/route27}] - ["/v2/public/datasets/:dataset-id" {:name :test/route28}] - ["/v2/users/:user-id/topics/bulk" {:name :test/route29}] - ["/v1/messages/device/:client-id" {:name :test/route30}] - ["/v1/users/:user-id/owned-orgs" {:name :test/route31}] - ["/v1/topics/:topic" {:name :test/route32}] - ["/v1/users/:user-id/bookmark/:topic" {:name :test/route33}] - ["/v1/orgs/:org-id/members/:user-id" {:name :test/route34}] - ["/v1/users/:user-id/devices/:client-id" {:name :test/route35}] - ["/v1/users/:user-id" {:name :test/route36}] - ["/v1/orgs/:org-id/devices" {:name :test/route37}] - ["/v1/orgs/:org-id/members" {:name :test/route38}] - ["/v2/orgs/:org-id/topics" {:name :test/route40}] - ["/v1/whoami" {:name :test/route41}] - ["/v1/orgs/:org-id" {:name :test/route42}] - ["/v1/users/:user-id/api-key" {:name :test/route43}] - ["/v2/schemas" {:name :test/route44}] - ["/v2/users/:user-id/topics" {:name :test/route45}] - ["/v1/orgs/:org-id/confirm-membership/:token" {:name :test/route46}] - ["/v2/topics/:topic" {:name :test/route47}] - ["/v1/messages/topic/:topic" {:name :test/route48}] - ["/v1/users/:user-id/devices/:client-id/reset-password" {:name :test/route49}] - ["/v2/topics" {:name :test/route50}] - ["/v1/login" {:name :test/route51}] - ["/v1/users/:user-id/orgs" {:name :test/route52}] - ["/v2/public/messages/dataset/:dataset-id" {:name :test/route53}] - ["/v1/topics" {:name :test/route54}] - ["/v1/orgs" {:name :test/route55}] - ["/v1/users/:user-id/bookmarks" {:name :test/route56}] - ["/v1/orgs/:org-id/topics" {:name :test/route57}]]) - -(comment - (require '[io.pedestal.http.route.prefix-tree :as p]) - (def tree-old (reduce (fn [acc [p d]] (p/insert acc p d)) nil routes)) - - ;; 2.3ms - (cc/quick-bench (dotimes [_ 1000] (p/lookup tree-old "/v1/orgs/1/topics")))) - -(comment - (def tree-new (reduce (fn [acc [p d]] (insert acc p d)) nil routes)) - - ;; 3.1ms - ;; 2.5ms (string equals) - ;; 2.5ms (protocol) - ;; 2.3ms (nil childs) - ;; 2.0ms (rando impros) - ;; 1.9ms (wild & catch shortcuts) - ;; 1.5ms (inline child fetching) - ;; 1.5ms (WildNode also backtracks) - ;; 1.4ms (precalculate segment-size) - ;; 1.3ms (fast-map) - ;; 1.3ms (dissoc wild & catch-all from children) - ;; 1.3ms (reified protocols) - ;; 0.8ms (flattened matching) - ;; 0.8ms (return route-data) - ;; 0.8ms (fix payloads) - (cc/quick-bench (dotimes [_ 1000] (lookup tree-new "/v1/orgs/1/topics" {})))) diff --git a/perf-test/clj/reitit/prefix_tree_perf_test.clj b/perf-test/clj/reitit/prefix_tree_perf_test.clj new file mode 100644 index 00000000..c607d675 --- /dev/null +++ b/perf-test/clj/reitit/prefix_tree_perf_test.clj @@ -0,0 +1,105 @@ +(ns reitit.prefix-tree-perf-test + (:require [clojure.test :refer :all] + [io.pedestal.http.route.prefix-tree :as p] + [reitit.trie :as trie] + [criterium.core :as cc])) + +;; +;; testing +;; + +(def routes + [["/v2/whoami" {:name :test/route1}] + ["/v2/users/:user-id/datasets" {:name :test/route2}] + ["/v2/public/projects/:project-id/datasets" {:name :test/route3}] + ["/v1/public/topics/:topic" {:name :test/route4}] + ["/v1/users/:user-id/orgs/:org-id" {:name :test/route5}] + ["/v1/search/topics/:term" {:name :test/route6}] + ["/v1/users/:user-id/invitations" {:name :test/route7}] + ["/v1/users/:user-id/topics" {:name :test/route9}] + ["/v1/users/:user-id/bookmarks/followers" {:name :test/route10}] + ["/v2/datasets/:dataset-id" {:name :test/route11}] + ["/v1/orgs/:org-id/usage-stats" {:name :test/route12}] + ["/v1/orgs/:org-id/devices/:client-id" {:name :test/route13}] + ["/v1/messages/user/:user-id" {:name :test/route14}] + ["/v1/users/:user-id/devices" {:name :test/route15}] + ["/v1/public/users/:user-id" {:name :test/route16}] + ["/v1/orgs/:org-id/errors" {:name :test/route17}] + ["/v1/public/orgs/:org-id" {:name :test/route18}] + ["/v1/orgs/:org-id/invitations" {:name :test/route19}] + ["/v1/users/:user-id/device-errors" {:name :test/route22}] + ["/v2/login" {:name :test/route23}] + ["/v1/users/:user-id/usage-stats" {:name :test/route24}] + ["/v2/users/:user-id/devices" {:name :test/route25}] + ["/v1/users/:user-id/claim-device/:client-id" {:name :test/route26}] + ["/v2/public/projects/:project-id" {:name :test/route27}] + ["/v2/public/datasets/:dataset-id" {:name :test/route28}] + ["/v2/users/:user-id/topics/bulk" {:name :test/route29}] + ["/v1/messages/device/:client-id" {:name :test/route30}] + ["/v1/users/:user-id/owned-orgs" {:name :test/route31}] + ["/v1/topics/:topic" {:name :test/route32}] + ["/v1/users/:user-id/bookmark/:topic" {:name :test/route33}] + ["/v1/orgs/:org-id/members/:user-id" {:name :test/route34}] + ["/v1/users/:user-id/devices/:client-id" {:name :test/route35}] + ["/v1/users/:user-id" {:name :test/route36}] + ["/v1/orgs/:org-id/devices" {:name :test/route37}] + ["/v1/orgs/:org-id/members" {:name :test/route38}] + ["/v2/orgs/:org-id/topics" {:name :test/route40}] + ["/v1/whoami" {:name :test/route41}] + ["/v1/orgs/:org-id" {:name :test/route42}] + ["/v1/users/:user-id/api-key" {:name :test/route43}] + ["/v2/schemas" {:name :test/route44}] + ["/v2/users/:user-id/topics" {:name :test/route45}] + ["/v1/orgs/:org-id/confirm-membership/:token" {:name :test/route46}] + ["/v2/topics/:topic" {:name :test/route47}] + ["/v1/messages/topic/:topic" {:name :test/route48}] + ["/v1/users/:user-id/devices/:client-id/reset-password" {:name :test/route49}] + ["/v2/topics" {:name :test/route50}] + ["/v1/login" {:name :test/route51}] + ["/v1/users/:user-id/orgs" {:name :test/route52}] + ["/v2/public/messages/dataset/:dataset-id" {:name :test/route53}] + ["/v1/topics" {:name :test/route54}] + ["/v1/orgs" {:name :test/route55}] + ["/v1/users/:user-id/bookmarks" {:name :test/route56}] + ["/v1/orgs/:org-id/topics" {:name :test/route57}]]) + +(def pedestal-tree + (reduce + (fn [acc [p d]] + (p/insert acc p d)) + nil routes)) + +(def reitit-tree + (reduce + (fn [acc [p d]] + (trie/insert acc p d)) + nil routes)) + +(defn bench! [] + + ;; 2.3ms + (cc/quick-bench + (dotimes [_ 1000] + (p/lookup pedestal-tree "/v1/orgs/1/topics"))) + + ;; 3.1ms + ;; 2.5ms (string equals) + ;; 2.5ms (protocol) + ;; 2.3ms (nil childs) + ;; 2.0ms (rando impros) + ;; 1.9ms (wild & catch shortcuts) + ;; 1.5ms (inline child fetching) + ;; 1.5ms (WildNode also backtracks) + ;; 1.4ms (precalculate segment-size) + ;; 1.3ms (fast-map) + ;; 1.3ms (dissoc wild & catch-all from children) + ;; 1.3ms (reified protocols) + ;; 0.8ms (flattened matching) + ;; 0.8ms (return route-data) + ;; 0.8ms (fix payloads) + (cc/quick-bench + (dotimes [_ 1000] + (trie/lookup reitit-tree "/v1/orgs/1/topics" {})))) + +(comment + (bench!)) From 2f6bfb33f8bf02a8999b0aa1bc0126f610139be4 Mon Sep 17 00:00:00 2001 From: Tommi Reiman Date: Sat, 11 Nov 2017 16:38:16 +0200 Subject: [PATCH 12/15] Use linear-router always if there are conflicting routes --- modules/reitit-core/src/reitit/core.cljc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/reitit-core/src/reitit/core.cljc b/modules/reitit-core/src/reitit/core.cljc index cec9c33d..afeae96a 100644 --- a/modules/reitit-core/src/reitit/core.cljc +++ b/modules/reitit-core/src/reitit/core.cljc @@ -350,10 +350,10 @@ router (cond router router (and (= 1 (count routes)) (not wilds?)) single-static-path-router + conflicting linear-router (not wilds?) lookup-router all-wilds? prefix-tree-router - (not conflicting) mixed-router - :else prefix-tree-router)] + :else mixed-router)] (when-let [conflicts (:conflicts opts)] (when conflicting (conflicts conflicting))) From 1f27021c29fc5bec5859224132ff6a12be1f6bb7 Mon Sep 17 00:00:00 2001 From: Tommi Reiman Date: Sat, 11 Nov 2017 16:39:11 +0200 Subject: [PATCH 13/15] Tries have a human-readable representation --- modules/reitit-core/src/reitit/trie.cljc | 27 +++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/modules/reitit-core/src/reitit/trie.cljc b/modules/reitit-core/src/reitit/trie.cljc index 5d47eb7a..a0a14776 100644 --- a/modules/reitit-core/src/reitit/trie.cljc +++ b/modules/reitit-core/src/reitit/trie.cljc @@ -21,7 +21,9 @@ (lookup [this path params]) (get-segment [this]) (update-segment [this subs lcs]) + (get-data [this]) (set-data [this data]) + (get-chidren [this]) (add-child [this key child]) (insert-child [this key path-spec data])) @@ -36,6 +38,7 @@ (let [?wild (maybe-wild-node children) ?catch (maybe-catch-all-node children) children' (impl/fast-map children)] + ^{:type ::node} (reify Node (lookup [_ path params] @@ -51,26 +54,36 @@ (->Match data (assoc params param path))))) (get-segment [_] segment) + (get-data [_] + data) (set-data [_ data] (wild-node segment param children data)) + (get-chidren [_] + children) (add-child [_ key child] (wild-node segment param (assoc children key child) data)) (insert-child [_ key path-spec child-data] (wild-node segment param (update children key insert path-spec child-data) data))))) (defn- catch-all-node [segment children param data] + ^{:type ::node} (reify Node (lookup [_ path params] (->Match data (assoc params param path))) (get-segment [_] - segment))) + segment) + (get-data [_] + data) + (get-chidren [_] + children))) (defn- static-node [^String segment children data] (let [size (count segment) ?wild (maybe-wild-node children) ?catch (maybe-catch-all-node children) children' (impl/fast-map children)] + ^{:type ::node} (reify Node (lookup [_ path params] @@ -87,8 +100,12 @@ segment) (update-segment [_ subs lcs] (static-node (subs segment lcs) children data)) + (get-data [_] + data) (set-data [_ data] (static-node segment children data)) + (get-chidren [_] + children) (add-child [_ key child] (static-node segment (assoc children key child) data)) (insert-child [_ key path-spec child-data] @@ -196,3 +213,11 @@ :else (split node path-spec data lcs)))))) + +(defn view + "Returns a view representation of a prefix-tree." + [x] + (vec (concat + [(get-segment x)] + (some->> (get-chidren x) vals seq (map view)) + (some->> (get-data x) vector)))) From 8bde6aefa26ca5ee88fe956746f66c3b495a3af1 Mon Sep 17 00:00:00 2001 From: Tommi Reiman Date: Sat, 11 Nov 2017 16:46:43 +0200 Subject: [PATCH 14/15] Test that different routers work correctly --- test/cljc/reitit/core_test.cljc | 190 +++++++++++--------------------- 1 file changed, 65 insertions(+), 125 deletions(-) diff --git a/test/cljc/reitit/core_test.cljc b/test/cljc/reitit/core_test.cljc index dbee0bac..5e770e1f 100644 --- a/test/cljc/reitit/core_test.cljc +++ b/test/cljc/reitit/core_test.cljc @@ -7,140 +7,80 @@ (deftest reitit-test - (testing "prefix-tree-router" - (let [router (r/router ["/api" ["/ipa" ["/:size" ::beer]]])] - (is (= :prefix-tree-router (r/router-name router))) - (is (= [["/api/ipa/:size" {:name ::beer} nil]] - (r/routes router))) - (is (= true (map? (r/options router)))) - (is (= (r/map->Match - {:template "/api/ipa/:size" - :meta {:name ::beer} - :path "/api/ipa/large" - :params {:size "large"}}) - (r/match-by-path router "/api/ipa/large"))) - (is (= (r/map->Match - {:template "/api/ipa/:size" - :meta {:name ::beer} - :path "/api/ipa/large" - :params {:size "large"}}) - (r/match-by-name router ::beer {:size "large"}))) - (is (= nil (r/match-by-name router "ILLEGAL"))) - (is (= [::beer] (r/route-names router))) - - (testing "name-based routing with missing parameters" - (is (= (r/map->PartialMatch + (testing "routers handling wildcard paths" + (are [r name] + (let [router (r/router ["/api" ["/ipa" ["/:size" ::beer]]] {:router r})] + (is (= name (r/router-name router))) + (is (= [["/api/ipa/:size" {:name ::beer} nil]] + (r/routes router))) + (is (= true (map? (r/options router)))) + (is (= (r/map->Match {:template "/api/ipa/:size" :meta {:name ::beer} - :required #{:size} - :params nil}) - (r/match-by-name router ::beer))) - (is (= true (r/partial-match? (r/match-by-name router ::beer)))) - (is (thrown-with-msg? - ExceptionInfo - #"^missing path-params for route /api/ipa/:size -> \#\{:size\}$" - (r/match-by-name! router ::beer)))))) - - (testing "linear-router" - (let [router (r/router ["/api" ["/ipa" ["/:size" ::beer]]] {:router r/linear-router})] - (is (= :linear-router (r/router-name router))) - (is (= [["/api/ipa/:size" {:name ::beer} nil]] - (r/routes router))) - (is (= true (map? (r/options router)))) - (is (= (r/map->Match - {:template "/api/ipa/:size" - :meta {:name ::beer} - :path "/api/ipa/large" - :params {:size "large"}}) - (r/match-by-path router "/api/ipa/large"))) - (is (= (r/map->Match - {:template "/api/ipa/:size" - :meta {:name ::beer} - :path "/api/ipa/large" - :params {:size "large"}}) - (r/match-by-name router ::beer {:size "large"}))) - (is (= nil (r/match-by-name router "ILLEGAL"))) - (is (= [::beer] (r/route-names router))) - - (testing "name-based routing with missing parameters" - (is (= (r/map->PartialMatch + :path "/api/ipa/large" + :params {:size "large"}}) + (r/match-by-path router "/api/ipa/large"))) + (is (= (r/map->Match {:template "/api/ipa/:size" :meta {:name ::beer} - :required #{:size} - :params nil}) - (r/match-by-name router ::beer))) - (is (= true (r/partial-match? (r/match-by-name router ::beer)))) - (is (thrown-with-msg? - ExceptionInfo - #"^missing path-params for route /api/ipa/:size -> \#\{:size\}$" - (r/match-by-name! router ::beer)))))) + :path "/api/ipa/large" + :params {:size "large"}}) + (r/match-by-name router ::beer {:size "large"}))) + (is (= nil (r/match-by-name router "ILLEGAL"))) + (is (= [::beer] (r/route-names router))) - (testing "lookup-router" - (let [router (r/router ["/api" ["/ipa" ["/large" ::beer]]] {:router r/lookup-router})] - (is (= :lookup-router (r/router-name router))) - (is (= [["/api/ipa/large" {:name ::beer} nil]] - (r/routes router))) - (is (= true (map? (r/options router)))) - (is (= (r/map->Match - {:template "/api/ipa/large" - :meta {:name ::beer} - :path "/api/ipa/large" - :params {}}) - (r/match-by-path router "/api/ipa/large"))) - (is (= (r/map->Match - {:template "/api/ipa/large" - :meta {:name ::beer} - :path "/api/ipa/large" - :params {:size "large"}}) - (r/match-by-name router ::beer {:size "large"}))) - (is (= nil (r/match-by-name router "ILLEGAL"))) - (is (= [::beer] (r/route-names router))) + (testing "name-based routing with missing parameters" + (is (= (r/map->PartialMatch + {:template "/api/ipa/:size" + :meta {:name ::beer} + :required #{:size} + :params nil}) + (r/match-by-name router ::beer))) + (is (= true (r/partial-match? (r/match-by-name router ::beer)))) + (is (thrown-with-msg? + ExceptionInfo + #"^missing path-params for route /api/ipa/:size -> \#\{:size\}$" + (r/match-by-name! router ::beer))))) - (testing "can't be created with wildcard routes" - (is (thrown-with-msg? - ExceptionInfo - #"can't create :lookup-router with wildcard routes" - (r/lookup-router - (r/resolve-routes - ["/api/:version/ping"] {}))))))) + r/linear-router :linear-router + r/prefix-tree-router :prefix-tree-router + r/mixed-router :mixed-router)) - (testing "single-static-path-router" - (let [router (r/router ["/api" ["/ipa" ["/large" ::beer]]])] - (is (= :single-static-path-router (r/router-name router))) - (is (= [["/api/ipa/large" {:name ::beer} nil]] - (r/routes router))) - (is (= true (map? (r/options router)))) - (is (= (r/map->Match - {:template "/api/ipa/large" - :meta {:name ::beer} - :path "/api/ipa/large" - :params {}}) - (r/match-by-path router "/api/ipa/large"))) - (is (= (r/map->Match - {:template "/api/ipa/large" - :meta {:name ::beer} - :path "/api/ipa/large" - :params {:size "large"}}) - (r/match-by-name router ::beer {:size "large"}))) - (is (= nil (r/match-by-name router "ILLEGAL"))) - (is (= [::beer] (r/route-names router))) + (testing "routers handling static paths" + (are [r name] + (let [router (r/router ["/api" ["/ipa" ["/large" ::beer]]] {:router r})] + (is (= name (r/router-name router))) + (is (= [["/api/ipa/large" {:name ::beer} nil]] + (r/routes router))) + (is (= true (map? (r/options router)))) + (is (= (r/map->Match + {:template "/api/ipa/large" + :meta {:name ::beer} + :path "/api/ipa/large" + :params {}}) + (r/match-by-path router "/api/ipa/large"))) + (is (= (r/map->Match + {:template "/api/ipa/large" + :meta {:name ::beer} + :path "/api/ipa/large" + :params {:size "large"}}) + (r/match-by-name router ::beer {:size "large"}))) + (is (= nil (r/match-by-name router "ILLEGAL"))) + (is (= [::beer] (r/route-names router))) - (testing "can't be created with wildcard routes" - (is (thrown-with-msg? - ExceptionInfo - #":single-static-path-router requires exactly 1 static route" - (r/single-static-path-router - (r/resolve-routes - ["/api/:version/ping"] {}))))) + (testing "can't be created with wildcard routes" + (is (thrown-with-msg? + ExceptionInfo + #"can't create :lookup-router with wildcard routes" + (r/lookup-router + (r/resolve-routes + ["/api/:version/ping"] {})))))) - (testing "can't be created with multiple routes" - (is (thrown-with-msg? - ExceptionInfo - #":single-static-path-router requires exactly 1 static route" - (r/single-static-path-router - (r/resolve-routes - [["/ping"] - ["/pong"]] {}))))))) + r/lookup-router :lookup-router + r/single-static-path-router :single-static-path-router + r/linear-router :linear-router + r/prefix-tree-router :prefix-tree-router + r/mixed-router :mixed-router)) (testing "route coercion & compilation" From 5390086d7f1bfcb796466f96836f37710fcaec2b Mon Sep 17 00:00:00 2001 From: Tommi Reiman Date: Sat, 11 Nov 2017 16:52:32 +0200 Subject: [PATCH 15/15] Fix docs --- doc/advanced/different_routers.md | 21 ++++++++++++++------- doc/advanced/route_validation.md | 2 +- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/doc/advanced/different_routers.md b/doc/advanced/different_routers.md index ae742760..98e01654 100644 --- a/doc/advanced/different_routers.md +++ b/doc/advanced/different_routers.md @@ -1,18 +1,25 @@ # Different Routers -Reitit ships with several different implementations for the `Router` protocol, originally based on the [Pedestal](https://github.com/pedestal/pedestal/tree/master/route) implementation. `router` selects the most suitable implementation by inspecting the expanded routes. The implementation can be set manually using `:router` option, see [configuring routers](advanced/configuring_routers.md). +Reitit ships with several different implementations for the `Router` protocol, originally based on the [Pedestal](https://github.com/pedestal/pedestal/tree/master/route) implementation. `router` function selects the most suitable implementation by inspecting the expanded routes. The implementation can be set manually using `:router` option, see [configuring routers](advanced/configuring_routers.md). | router | description | | ------------------------------|-------------| -| `:linear-router` | Matches the routes one-by-one starting from the top until a match is found. Works with any kind of routes. -| `:lookup-router` | Fast router, uses hash-lookup to resolve the route. Valid if no paths have path or catch-all parameters. -| `:mixed-router` | Creates internally a `:linear-router` and a `:lookup-router` and used them to effectively get best-of-both-worlds. Valid if there are no [Route conflicts](../basics/route_conflicts.md). -| `::single-static-path-router` | Fastest possible router: valid only if there is one static route. -| `:prefix-tree-router` | TODO: https://github.com/julienschmidt/httprouter#how-does-it-work +| `:linear-router` | Matches the routes one-by-one starting from the top until a match is found. Works with any kind of routes. Slow, but works with all route trees. +| `:lookup-router` | Fast router, uses hash-lookup to resolve the route. Valid if no paths have path or catch-all parameters and there are no [Route conflicts](../basics/route_conflicts.md). +| `:mixed-router` | Creates internally a `:prefix-tree-router` and a `:lookup-router` and used them to effectively get best-of-both-worlds. Valid only if there are no [Route conflicts](../basics/route_conflicts.md). +| `::single-static-path-router` | Super fast router: sting-matches the route. Valid only if there is one static route. +| `:prefix-tree-router` | Router that creates a [prefix-tree](https://en.wikipedia.org/wiki/Radix_tree) out of an route table. Much faster than `:linear-router`. Valid only if there are no [Route conflicts](../basics/route_conflicts.md). -The router name can be asked from the router +The router name can be asked from the router: ```clj +(require '[reitit.core :as r]) + +(def router + (r/router + [["/ping" ::ping] + ["/api/:users" ::users]])) + (r/router-name router) ; :mixed-router ``` diff --git a/doc/advanced/route_validation.md b/doc/advanced/route_validation.md index 80ef1bcd..90a84222 100644 --- a/doc/advanced/route_validation.md +++ b/doc/advanced/route_validation.md @@ -5,7 +5,7 @@ Namespace `reitit.spec` contains [clojure.spec](https://clojure.org/about/spec) **NOTE:** Use of specs requires to use one of the following: * `[org.clojure/clojurescript "1.9.660"]` (or higher) -* `[org.clojure/clojure "1.9.0-beta2"]` (or higher) +* `[org.clojure/clojure "1.9.0-RC1"]` (or higher) * `[clojure-future-spec "1.9.0-alpha17"]` (if Clojure 1.8 is used) ## Example