Skip to content

Commit f3c183a

Browse files
committed
Remove logging of cache misses
Might resolve this later, thus the tests are just commented out.
1 parent 2023389 commit f3c183a

File tree

4 files changed

+27
-31
lines changed

4 files changed

+27
-31
lines changed

CHANGELOG.md

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,14 @@
1+
## 0.24.3 (13.12.2015)
2+
3+
* coercer-cache is now per Route instead beeing global and based on a FIFO size 100 cache. Avoids potential memory
4+
leaks when using anonymous coercion matchers (which never hit the cache).
5+
6+
* Updated deps:
7+
8+
```clj
9+
[prismatic/schema "1.0.4"] is available but we use "1.0.3"
10+
```
11+
112
## 0.24.2 (8.12.2015)
213

314
**[compare](https://github.com/metosin/compojure-api/compare/0.24.1...0.24.2)**

project.clj

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
[potemkin "0.4.2"]
1010
[cheshire "5.5.0"]
1111
[compojure "1.4.0"]
12-
[prismatic/schema "1.0.3"]
12+
[prismatic/schema "1.0.4"]
1313
[org.tobereplaced/lettercase "1.0.0"]
1414
[frankiesardo/linked "1.2.6"]
1515
[metosin/ring-http-response "0.6.5"]

src/compojure/api/meta.clj

Lines changed: 12 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -58,32 +58,18 @@
5858
memoized version of the function keeps a cache of the mapping from arguments
5959
to results and, when calls with the same arguments are repeated often, has
6060
higher performance at the expense of higher memory use. FIFO with 100 entries.
61-
If the cached if filled, there will be a WARNING logged at least once -> root cause
62-
is most propably that the route is using anonymous coercer, which doesn't hit
63-
the cache. It will produce slower performance, but works otherwise as expected."
64-
[name]
65-
(let [cache (atom {:mem (linked/map), :overflow false})
61+
Cache will be filled if anonymous coercers are used (does not match the cache)"
62+
[]
63+
(let [cache (atom (linked/map))
6664
cache-size 100]
6765
(fn [& args]
68-
(or (-> @cache :mem (get args))
66+
(or (@cache args)
6967
(let [coercer (apply sc/coercer args)]
70-
(swap! cache (fn [cache]
71-
(let [mem (assoc (:mem cache) args coercer)]
68+
(swap! cache (fn [mem]
69+
(let [mem (assoc mem args coercer)]
7270
(if (>= (count mem) cache-size)
73-
(do
74-
(when-not (:overflow cache)
75-
;; side-effecting within a swap! might cause multiple writes.
76-
;; it's ok'ish as we are just reporting something that should be
77-
;; fixes at development time
78-
(logging/log! :warning (str "Coercion memoization cache for " name
79-
" maxing at " cache-size ". "
80-
"You might recreate the coercer "
81-
"matcher on each request, causing "
82-
"coercer re-compilation per request, "
83-
"effecting coercion performance.")))
84-
{:mem (dissoc mem (-> mem first first))
85-
:overflow true})
86-
(assoc cache :mem mem)))))
71+
(dissoc mem (-> mem first first))
72+
mem))))
8773
coercer)))))
8874

8975
(defn strict [schema]
@@ -152,9 +138,9 @@
152138
;;
153139

154140
(defmulti restructure-param
155-
"Restructures a key value pair in smart routes. By default the key
156-
is consumed form the :parameters map in acc. k = given key, v = value."
157-
(fn [k v acc] k))
141+
"Restructures a key value pair in smart routes. By default the key
142+
is consumed form the :parameters map in acc. k = given key, v = value."
143+
(fn [k v acc] k))
158144

159145
;;
160146
;; Pass-through swagger metadata
@@ -358,7 +344,6 @@
358344
(defn restructure [method [path arg & args] & [{:keys [body-wrap]}]]
359345
(let [body-wrap (or body-wrap 'do)
360346
method-symbol (symbol (str (-> method meta :ns) "/" (-> method meta :name)))
361-
coercer-name (str (keyword (.toLowerCase (name method-symbol))) " " path)
362347
[parameters body] (extract-parameters args)
363348
[lets letks responses middlewares] [[] [] nil nil]
364349
[lets arg-with-request arg] (destructure-compojure-api-request lets arg)
@@ -375,7 +360,7 @@
375360
(map-of lets letks responses middlewares parameters body)
376361
parameters)
377362

378-
pre-lets [+compojure-api-coercer+ `(memoized-coercer ~coercer-name)]
363+
pre-lets [+compojure-api-coercer+ `(memoized-coercer)]
379364

380365
body `(~body-wrap ~@body)
381366
body (if (seq letks) `(letk ~letks ~body) body)

test/compojure/api/coercion_test.clj

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -115,9 +115,9 @@
115115

116116
(fact "anonymous matchers, with 100+ calls to same endpoint"
117117

118-
(fact "at api-level, matcher is reused and the coercion matcher cache is not filled"
118+
#_(fact "at api-level, matcher is reused and the coercion matcher cache is not filled"
119119
(let [app (api
120-
{:coercion (assoc mw/default-coercion-matchers :string (constantly nil))}
120+
{:coercion (constantly mw/default-coercion-matchers)}
121121
(GET* "/anonymous" []
122122
:query-params [i :- s/Str]
123123
(ok {:i i})))]
@@ -129,7 +129,7 @@
129129
(provided
130130
(compojure.api.impl.logging/log! & anything) => irrelevant :times 0)))
131131

132-
(fact "at route-level, matcher is NOT reused and the the coercion matcher cache is filled"
132+
#_(fact "at route-level, matcher is NOT reused and the the coercion matcher cache is filled"
133133
(let [app (api
134134
(GET* "/anonymous" []
135135
:coercion (constantly (assoc mw/default-coercion-matchers :string (constantly nil)))

0 commit comments

Comments
 (0)