Fork me on GitHub
#clojure-dev
<
2020-08-06
>
Vincent Cantin10:08:24

I noticed the implementation of clojure.core/update-in could me made simpler by not passing the f and args to the up function. Do you think it is worth changing it?

(defn update-in
  "'Updates' a value in a nested associative structure, where ks is a
  sequence of keys and f is a function that will take the old value
  and any supplied args and return the new value, and returns a new
  nested structure.  If any levels do not exist, hash-maps will be
  created."
  {:added "1.0"
   :static true}
  ([m ks f & args]
     (let [up (fn up [m ks f args]
                (let [[k & ks] ks]
                  (if ks
                    (assoc m k (up (get m k) ks f args))
                    (assoc m k (apply f (get m k) args)))))]
       (up m ks f args))))

Vincent Cantin10:08:58

It could be

(defn update-in
  "'Updates' a value in a nested associative structure, where ks is a
  sequence of keys and f is a function that will take the old value
  and any supplied args and return the new value, and returns a new
  nested structure.  If any levels do not exist, hash-maps will be
  created."
  {:added "1.0"
   :static true}
  ([m ks f & args]
   (let [up (fn up [m ks]
              (let [[k & ks] ks]
                (if ks
                  (assoc m k (up (get m k) ks f args))
                  (assoc m k (apply f (get m k) args)))))]
     (up m ks))))

mpenet11:08:20

@vincent.cantin did you profile it? I suspect with args coming from the closure the jvm might not be able to cache the up fn (it's pure in the original)

Vincent Cantin11:08:29

I did not profile it. You might be right.

mpenet11:08:59

you'd get 1 fn instance per call vs 1 for all update-in calls

Vincent Cantin11:08:17

Yes, it makes sense. Thank you.