From fe0ea19e31acc3b92b1eb28c8fc4825fc2736eda Mon Sep 17 00:00:00 2001
From: Tommi Reiman <tommi@iki.fi>
Date: Sun, 27 Jan 2019 18:57:29 +0200
Subject: [PATCH] Better perf with transient parameters

---
 modules/reitit-core/java-src/reitit/Trie.java | 23 +++++++++++--------
 modules/reitit-core/src/reitit/core.cljc      |  2 +-
 modules/reitit-core/src/reitit/trie.cljc      |  2 +-
 .../clj/reitit/opensensors_perf_test.clj      | 23 +++++++++++++++++--
 project.clj                                   |  2 ++
 5 files changed, 39 insertions(+), 13 deletions(-)

diff --git a/modules/reitit-core/java-src/reitit/Trie.java b/modules/reitit-core/java-src/reitit/Trie.java
index 4dfbae2b..aa12c957 100644
--- a/modules/reitit-core/java-src/reitit/Trie.java
+++ b/modules/reitit-core/java-src/reitit/Trie.java
@@ -2,7 +2,10 @@ package reitit;
 
 // https://www.codeproject.com/Tips/1190293/Iteration-Over-Java-Collections-with-High-Performa
 
+import clojure.lang.IPersistentMap;
+import clojure.lang.ITransientMap;
 import clojure.lang.Keyword;
+import clojure.lang.PersistentArrayMap;
 
 import java.io.UnsupportedEncodingException;
 import java.net.URLDecoder;
@@ -22,14 +25,18 @@ public class Trie {
   }
 
   public static class Match {
-    public final List<Object> params = new ArrayList<>();
+    final ITransientMap params = PersistentArrayMap.EMPTY.asTransient();
     public Object data;
 
+    public IPersistentMap parameters() {
+      return params.persistent();
+    }
+
     @Override
     public String toString() {
       Map<Object, Object> m = new HashMap<>();
       m.put(Keyword.intern("data"), data);
-      m.put(Keyword.intern("params"), params);
+      m.put(Keyword.intern("params"), params.persistent());
       return m.toString();
     }
   }
@@ -132,8 +139,7 @@ public class Trie {
           if (value[j] == '/') {
             final Match m = child.match(j, path, match);
             if (m != null) {
-              m.params.add(key);
-              m.params.add(decode(value, i, j - i, hasPercent, hasPlus));
+              m.params.assoc(key, decode(value, i, j - i, hasPercent, hasPlus));
             }
             return m;
           } else if (value[j] == '%') {
@@ -142,12 +148,11 @@ public class Trie {
             hasPlus = true;
           }
         }
-        if (child instanceof DataMatcher) {
-          final Match m = child.match(path.size, path, match);
-          m.params.add(key);
-          m.params.add(decode(value, i, path.size - i, hasPercent, hasPlus));
-          return m;
+        final Match m = child.match(path.size, path, match);
+        if (m != null) {
+          m.params.assoc(key, decode(value, i, path.size - i, hasPercent, hasPlus));
         }
+        return m;
       }
       return null;
     }
diff --git a/modules/reitit-core/src/reitit/core.cljc b/modules/reitit-core/src/reitit/core.cljc
index 5df916af..d6cc99b1 100644
--- a/modules/reitit-core/src/reitit/core.cljc
+++ b/modules/reitit-core/src/reitit/core.cljc
@@ -2,7 +2,7 @@
   (:require [meta-merge.core :refer [meta-merge]]
             [clojure.string :as str]
             [reitit.segment :as segment]
-            [reitit.segment :as trie]
+            [reitit.trie :as trie]
             [reitit.impl :as impl #?@(:cljs [:refer [Route]])])
   #?(:clj
      (:import (reitit.impl Route))))
diff --git a/modules/reitit-core/src/reitit/trie.cljc b/modules/reitit-core/src/reitit/trie.cljc
index 1fa98100..f615a597 100644
--- a/modules/reitit-core/src/reitit/trie.cljc
+++ b/modules/reitit-core/src/reitit/trie.cljc
@@ -106,7 +106,7 @@
 
 (defn lookup [^Trie$Matcher matcher path]
   (if-let [match ^Trie$Match (Trie/lookup matcher ^String path)]
-    (->Match (.data match) (clojure.lang.PersistentHashMap/create (.toArray (.params match))))))
+    (->Match (.data match) (.parameters match))))
 
 ;;
 ;; matcher
diff --git a/perf-test/clj/reitit/opensensors_perf_test.clj b/perf-test/clj/reitit/opensensors_perf_test.clj
index f8ce7900..e742422d 100644
--- a/perf-test/clj/reitit/opensensors_perf_test.clj
+++ b/perf-test/clj/reitit/opensensors_perf_test.clj
@@ -568,6 +568,7 @@
     ;;   662ns (prefix-tree-router)
     ;;   567ns (segment-router)
     ;;   326ns (java-segment-router)
+    ;;   194ms (trie)
     (b! "reitit" reitit-f)
 
     ;;  2845ns
@@ -578,10 +579,12 @@
     ;;   806ns (decode path-parameters)
     ;;   735ns (maybe-map-values)
     ;;   474ns (java-segment-router)
-    #_(b! "reitit-ring" reitit-ring-f)
+    ;;   373ms (trie)
+    (b! "reitit-ring" reitit-ring-f)
 
     ;;   385ns (java-segment-router, no injects)
-    #_(b! "reitit-ring-fast" reitit-ring-fast-f)
+    ;;   271ms (trie)
+    (b! "reitit-ring-fast" reitit-ring-fast-f)
 
     ;;  2553ns (linear-router)
     ;;   630ns (segment-router-backed)
@@ -611,3 +614,19 @@
 (comment
   (bench-rest!))
 
+(set! *warn-on-reflection* true)
+
+(require '[clj-async-profiler.core :as prof])
+
+(comment
+  ;; 629ms (arraylist)
+  ;; 395ns (transient)
+  (let [app (ring/ring-handler (ring/router opensensors-routes))]
+    (doseq [[p r] (-> app (ring/get-router) (r/routes))]
+      (when-not (app {:uri p, :request-method :get})
+        (println "FAIL:" p)))
+    (println (app {:uri "/v1/users/1/devices/1", :request-method :get}))
+    (prof/start {})
+    (dotimes [_ 100000]
+      (app {:uri "/v1/users/1/devices/1", :request-method :get}))
+    (str (prof/stop {}))))
diff --git a/project.clj b/project.clj
index f67ae638..b9e2b8b4 100644
--- a/project.clj
+++ b/project.clj
@@ -91,6 +91,8 @@
                                   [manifold "0.1.8"]
                                   [funcool/promesa "1.9.0"]
 
+                                  [com.clojure-goes-fast/clj-async-profiler "0.2.2"]
+
                                   ;; https://github.com/bensu/doo/issues/180
                                   [fipp "0.6.14" :exclusions [org.clojure/core.rrb-vector]]]}
              :1.9 {:dependencies [[org.clojure/clojure "1.9.0"]]}