clj-commons / potemkin

some ideas which are almost good
572 stars 53 forks source link

(keys (LazyMap. {:a (delay "B")})) will eagerly instantiate MapEntrys #23

Closed savagematt closed 10 years ago

savagematt commented 10 years ago

Not a bug so much as an observation. LazyMap (which is what I was using def-map-type for) isn't quite as lazy as it looks.

clojure.core/keys will eagerly create all MapEntrys. It may be worth pointing that out in the docs?

Is there any way to fix the problem? There's some magic in clojure.core's implementation of keys that I don't understand yet.

(def-map-type LazyMap [m]
              (get [_ k default-value]
                   (if (contains? m k)
                     (let [v (get m k)]
                       (if (instance? clojure.lang.Delay v)
                         @v
                         v))
                     default-value))
              (assoc [_ k v]
                (LazyMap. (assoc m k v)))
              (dissoc [_ k]
                      (LazyMap. (dissoc m k)))
              (keys [_]
                    (keys m)))

(let [was-called (atom false)
      d (delay (reset! was-called true))
      m (LazyMap. {:d d})]

; contains is ok
  (contains? m :d)
  (println @was-called) ; => false

; potemkin.collections/keys* is also well-behaved
  (doall (potemkin.collections/keys* m))
  (println @was-called)  ; => false

; .keySet as well
  (doall (.keySet m))
  (println @was-called) ; => false

; clojure.core/keys* not so much
  (doall (keys m))
  (println @was-called))  ; => true
ztellman commented 10 years ago

Hmmm. This appears to be due to how clojure.lang.RT/keys is defined:

static public ISeq keys(Object coll){
    return APersistentMap.KeySeq.create(seq(coll));
}

It takes all the entries, just to get at the keys. I can make the val lookup on each entry lazy, though, which I think is the semantically "right" way to give ourselves indirection. Thanks for pointing this out, I had completely missed this.

savagematt commented 10 years ago

No wonder I couldn't figure it out. I was looking at trunk, where I think keys might have changed implementation. Stared at that for the longest time. Thanks.

Are you saying that the MapEntrys that come back from seq and entryAt should overload val and getValue to lazily call valAt?

ztellman commented 10 years ago

Yeah, that was my thought.

On Thu, Jul 10, 2014 at 12:38 PM, savagematt notifications@github.com wrote:

No wonder I couldn't figure it out. I was looking at trunk, where I think keys might have changed implementation. Stared at that for the longest time. Thanks.

Are you saying that the MapEntrys that come back from seq and entryAt should overload val and getValue to lazily call valAt?

— Reply to this email directly or view it on GitHub https://github.com/ztellman/potemkin/issues/23#issuecomment-48653524.

ztellman commented 10 years ago

Merged, sorry for the delay.