From ec2076e38c43f04bbe1e78592d72089deb6dbdda Mon Sep 17 00:00:00 2001 From: Viktor Date: Fri, 22 Jan 2021 21:49:48 +0100 Subject: [PATCH 1/7] Fix some random spelling mistakes. Left behind by me last time I was in here, sry! --- src/selmer/tags.clj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/selmer/tags.clj b/src/selmer/tags.clj index 5f1a74d..e8ff076 100644 --- a/src/selmer/tags.clj +++ b/src/selmer/tags.clj @@ -205,7 +205,7 @@ (render-if render context-map condition success failure)))) (defn if-handler [params tag-content render rdr] - ; The main idea of this function is to genreate a list of test conditions and corresponding contetn, + ; The main idea of this function is to generate a list of test conditions and corresponding content, ; then going though them in order until a test is successful, and then returning the contents belonging to ; that test. From 12bece7b80afe3c1df9fd710a1213fd913538155 Mon Sep 17 00:00:00 2001 From: Viktor Date: Fri, 22 Jan 2021 21:36:26 +0100 Subject: [PATCH 2/7] Add a benchmark for filter accesses like {% if a.b.c.d %} There seems to be some performance problems here and this will make it easier to spot these. Currently runs in ~145ms: ``` BENCH: Many . acceses in an if clause Evaluation count : 6 in 6 samples of 1 calls. Execution time mean : 145.789759 ms Execution time std-deviation : 3.397597 ms Execution time lower quantile : 140.769053 ms ( 2.5%) Execution time upper quantile : 149.232308 ms (97.5%) Overhead used : 7.470498 ns ``` --- test/selmer/benchmark.clj | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/test/selmer/benchmark.clj b/test/selmer/benchmark.clj index d6aaee4..6ad77ec 100644 --- a/test/selmer/benchmark.clj +++ b/test/selmer/benchmark.clj @@ -3,10 +3,10 @@ [selmer.parser :refer :all] [selmer.filters :as filters] [selmer.util :refer :all] - [criterium.core :as criterium]) - (:import java.io.StringReader)) + [criterium.core :as criterium] + [clojure.string :as string])) -(def user (repeat 10 [{:name "test" }])) +(def user (repeat 10 [{:name "test"}])) (def nested-for-context {:users (repeat 10 user)}) @@ -48,3 +48,9 @@ (filters/add-filter! :inc (fn [^String s] (str (inc (Integer/parseInt s))))) (criterium/quick-bench (render (str "{{bar" filter-chain "}}") {:bar "0"}))) + +(deftest ^:benchmark if-bench + (println "BENCH: Many . acceses in an if clause") + (criterium/quick-bench + (render (string/join "" (repeat 1000 "{% if p.a.a.a.a %}{% endif %}")) + {}))) From aaabaf856ce23dcf8dc34dc6093ff4d849764956 Mon Sep 17 00:00:00 2001 From: Viktor Date: Fri, 22 Jan 2021 21:37:31 +0100 Subject: [PATCH 3/7] Speed up filter accesses like {% if a.b.c.d %}. The if-bench benchmark now goes down from ~145ms to ~30ms. The main culpit was fix-accessor, which was using exception handling to parse integers. Exceptions are expensive, at least when you use them a couple of thusand times: ``` ((time (dotimes [_ 100000] (try (Long/valueOf "nan") (catch NumberFormatException _)))) "Elapsed time: 219.507474 msecs" (time (dotimes [_ 100000] (when (re-matches #"\d+" "nan") (Long/valueOf "nan")))) "Elapsed time: 11.263704 msecs" ``` before: ``` BENCH: Many . acceses in an if clause Evaluation count : 6 in 6 samples of 1 calls. Execution time mean : 145.789759 ms Execution time std-deviation : 3.397597 ms Execution time lower quantile : 140.769053 ms ( 2.5%) Execution time upper quantile : 149.232308 ms (97.5%) Overhead used : 7.470498 ns ``` after: ``` BENCH: Many . acceses in an if clause Evaluation count : 30 in 6 samples of 5 calls. Execution time mean : 27.158986 ms Execution time std-deviation : 7.895735 ms Execution time lower quantile : 21.507726 ms ( 2.5%) Execution time upper quantile : 38.751057 ms (97.5%) Overhead used : 7.470498 ns ``` --- src/selmer/util.clj | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/selmer/util.clj b/src/selmer/util.clj index 9075a99..c73a2a5 100644 --- a/src/selmer/util.clj +++ b/src/selmer/util.clj @@ -228,14 +228,16 @@ (alter-var-root #'*missing-value-formatter* (constantly missing-value-fn)) (alter-var-root #'*filter-missing-values* (constantly filter-missing-values))) +(defn- parse-long [^String s] + (when (re-matches #"\d+" s) + (Long/valueOf s))) + (defn fix-accessor "Turns strings into keywords and strings like \"0\" into Longs so it can access vectors as well as maps." [ks] (mapv (fn [^String s] - (try (Long/valueOf s) - (catch NumberFormatException _ - (keyword s)))) + (or (parse-long s) (keyword s))) ks)) (defn parse-accessor From 9a167fbe553e254ff1b32e29b633a552f06fc760 Mon Sep 17 00:00:00 2001 From: Viktor Date: Mon, 25 Jan 2021 20:32:48 +0100 Subject: [PATCH 4/7] Add a benchmark for for-loops with if-clauses in them. This now runs in about 150ms. --- test/selmer/benchmark.clj | 11 ++++++++++- test/templates/numerics.html | 4 ++++ 2 files changed, 14 insertions(+), 1 deletion(-) create mode 100644 test/templates/numerics.html diff --git a/test/selmer/benchmark.clj b/test/selmer/benchmark.clj index 6ad77ec..9986a63 100644 --- a/test/selmer/benchmark.clj +++ b/test/selmer/benchmark.clj @@ -4,7 +4,8 @@ [selmer.filters :as filters] [selmer.util :refer :all] [criterium.core :as criterium] - [clojure.string :as string])) + [clojure.string :as string] + [selmer.parser :as parser])) (def user (repeat 10 [{:name "test"}])) @@ -54,3 +55,11 @@ (criterium/quick-bench (render (string/join "" (repeat 1000 "{% if p.a.a.a.a %}{% endif %}")) {}))) + +(deftest ^:benchmark many-numeric-if-clauses-bench + (println "BENCH: for loop with a numeric if clause in it") + (reset! parser/templates {}) + (cache-on!) + (render-file "templates/numerics.html" {:ps []}) + (criterium/quick-bench + (render-file "templates/numerics.html" {:ps (repeat 10000 "x")}))) diff --git a/test/templates/numerics.html b/test/templates/numerics.html new file mode 100644 index 0000000..8f83cad --- /dev/null +++ b/test/templates/numerics.html @@ -0,0 +1,4 @@ +{% for p in ps %} + {% if p.a > p.b %} + {% endif %} +{% endfor %} From 0406b5b79a6146e472dc71f69d08a4941952abae Mon Sep 17 00:00:00 2001 From: Viktor Date: Mon, 25 Jan 2021 20:54:34 +0100 Subject: [PATCH 5/7] Optimise if conditions like {% if a.b > a.c %}. By caching the parsing of the a.b style "filters", we speed up templates with lots of these quite a lot. The many-numeric-if-clauses-bench now runs about 3 times faster, from ~150ms to ~50ms on my machine. I'm not sure if it could be simplifed a bit, looks a bit complicated now. --- src/selmer/tags.clj | 36 +++++++++++++++++++----------------- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/src/selmer/tags.clj b/src/selmer/tags.clj index e8ff076..d86c81c 100644 --- a/src/selmer/tags.clj +++ b/src/selmer/tags.clj @@ -121,23 +121,25 @@ [#(comparator (parse-double %) (parse-double p2)) p1 nil]))) (defn numeric-expression-evaluation [[comparator context-key1 context-key2]] - (fn [context-map] - (let [[value1 value2] - (cond - (and context-key1 context-key2) - [(not-empty ((compile-filter-body context-key1) context-map)) - (not-empty ((compile-filter-body context-key2) context-map))] - context-key1 - [(not-empty ((compile-filter-body context-key1) context-map))] - context-key2 - [(not-empty ((compile-filter-body context-key2) context-map))])] - (cond - (and value1 value2) - (comparator value1 value2) - value1 - (comparator value1) - value2 - (comparator value2))))) + (let [l (when context-key1 (compile-filter-body context-key1)) + r (when context-key2 (compile-filter-body context-key2))] + (fn [context-map] + (let [[value1 value2] + (cond + (and context-key1 context-key2) + [(not-empty (l context-map)) + (not-empty (r context-map))] + context-key1 + [(not-empty (l context-map))] + context-key2 + [(not-empty (r context-map))])] + (cond + (and value1 value2) + (comparator value1 value2) + value1 + (comparator value1) + value2 + (comparator value2)))))) (defn if-any-all-fn [op params] (let [filters (map compile-filter-body params)] From 283c88f603546099381cf205a717a302284fd520 Mon Sep 17 00:00:00 2001 From: Viktor Date: Tue, 26 Jan 2021 09:57:08 +0100 Subject: [PATCH 6/7] Refactor: Simplify the code for numeric-expression-evaluation. I'm pretty sure this is right and not breaking anything, at least the tests pass still. --- src/selmer/tags.clj | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/src/selmer/tags.clj b/src/selmer/tags.clj index d86c81c..2e18b1a 100644 --- a/src/selmer/tags.clj +++ b/src/selmer/tags.clj @@ -121,23 +121,20 @@ [#(comparator (parse-double %) (parse-double p2)) p1 nil]))) (defn numeric-expression-evaluation [[comparator context-key1 context-key2]] + ; Parse the filter bodies first and close over them. + ; This makes them cached. (let [l (when context-key1 (compile-filter-body context-key1)) r (when context-key2 (compile-filter-body context-key2))] (fn [context-map] - (let [[value1 value2] - (cond - (and context-key1 context-key2) - [(not-empty (l context-map)) - (not-empty (r context-map))] - context-key1 - [(not-empty (l context-map))] - context-key2 - [(not-empty (r context-map))])] + (let [value1 (when context-key1 (not-empty (l context-map))) + value2 (when context-key2 (not-empty (r context-map)))] (cond (and value1 value2) (comparator value1 value2) + value1 (comparator value1) + value2 (comparator value2)))))) From 09bf2d67c59884dd6e86bbc3585a91773c1e2bd2 Mon Sep 17 00:00:00 2001 From: Viktor Date: Tue, 26 Jan 2021 17:38:49 +0100 Subject: [PATCH 7/7] Simplify and slighly speed up if-any-all claues. The newly added many-any-if-clauses-bench gets sped up by about 30-40%, by allowing short-circuiting. --- src/selmer/tags.clj | 8 ++++++-- test/selmer/benchmark.clj | 8 ++++++++ test/templates/any.html | 3 +++ 3 files changed, 17 insertions(+), 2 deletions(-) create mode 100644 test/templates/any.html diff --git a/src/selmer/tags.clj b/src/selmer/tags.clj index 2e18b1a..75b79c5 100644 --- a/src/selmer/tags.clj +++ b/src/selmer/tags.clj @@ -139,9 +139,13 @@ (comparator value2)))))) (defn if-any-all-fn [op params] + ; op is either the function "some" or "any" (let [filters (map compile-filter-body params)] - (fn [context-map] - (op #{true} (map #(if-result (% context-map)) filters))))) + (fn if-any-all-runtime-test [context-map] + ; We want to short-circuit here, in case + ; the first arg is true for ANY, or false for ALL. + (op (fn [f] (-> context-map (f) (if-result))) + filters)))) (defn parse-eq-arg [^String arg-string] (cond diff --git a/test/selmer/benchmark.clj b/test/selmer/benchmark.clj index 9986a63..e136f88 100644 --- a/test/selmer/benchmark.clj +++ b/test/selmer/benchmark.clj @@ -63,3 +63,11 @@ (render-file "templates/numerics.html" {:ps []}) (criterium/quick-bench (render-file "templates/numerics.html" {:ps (repeat 10000 "x")}))) + +(deftest ^:benchmark many-any-if-clauses-bench + (println "BENCH: for loop with a if any clause in it") + (reset! parser/templates {}) + (cache-on!) + (render-file "templates/any.html" {:products []}) + (criterium/quick-bench + (render-file "templates/any.html" {:products (repeat 10000 {})}))) diff --git a/test/templates/any.html b/test/templates/any.html new file mode 100644 index 0000000..604b723 --- /dev/null +++ b/test/templates/any.html @@ -0,0 +1,3 @@ +{% for p in products %} + {% if any p.a p.b p.c p.d p.e p.f p.g %}{% endif %} +{% endfor %}