Welcome! Please see the About page for a little more info on how this works.

+3 votes
in core.cache by

I'm using core.cache in combination with LazyMap. Lazy maps are map-like objects (new data type encapsulating maps and adding some logic to auto-force delayed values associated with keys).

When I started putting lazy maps as values of FIFO Cache I observed that all values are being realized just after the map lands in cache. After some digging I've found that it is caused by expression (= ::expired v) in clojure.core.cache.wrapped/lookup-or-miss.

What happens is that = internally calls clojure.lang.Util/equiv (since LazyMap implements IPersistentCollection) and then, after some dispatching, equiv is called to check whether the contents of a lazy map equals to other object (implicitly other map). This is the desired behavior and lazy map realizes all values to make comparison possible.

However, when comparing a map-like object to something which is not a map (nor a collection), short-circuit would be welcome as soon as we know that it will for sure return false. But changing that is probably more related to Clojure itself and things like CLJ-1375.

What could be done on core.cache's side, if that's not too problematic, would be a replacement of

(= ::expired v) with (identical? ::expired v)

in clojure.core.cache.wrapped.

It would not only fix corner cases like mine but also speed things up a bit (explicit referential equality rocks when it comes to Clojure keywords).

Other expressions where that could be changed (just in case) are:
(= ::nope ret) and (= ::nil v) from clojure.core.cache.

2 Answers

+1 vote
by

I can't see why changing all of those keyword comparisons to identical?checks would be a problem. I will noodle this a bit more, but in the meantime I've created https://clojure.atlassian.net/browse/CCACHE-66.

Do you have a reproducing example that I could look at?

by
Thank you!

Here is some example:


(require '[clojure.core.cache :as cache]
                 '[clojure.core.cache.wrapped :as cwr]
                 '[lazy-map.core :as lm])

(def C (atom (cache/fifo-cache-factory {})))
(def m (lm/->LazyMap {:a 1 :b (delay 8)}))

m
; => {:a 1, :b <unrealized>}

 (cwr/lookup-or-miss C :k (constantly m))
; => {:a 1, :b 8}

m
; => {:a 1, :b 8}


Requires:

com.intuitiveexplanations/lazy-map   {:mvn/version "1.0.0"}
org.clojure/core.cache                             {:mvn/version "1.0.225"}
+1 vote
by

Attaching patch for CCACHE-66:

https://gist.github.com/siefca/fa2745999281440867de1d1a89243f6d

(I've signed Clojure contributor's agreement about 2 or 3 years ago.)

...