Clojurians Log v2

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))))

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))))

@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)

I did not profile it. You might be right.

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

Yes, it makes sense. Thank you.

@vincent.cantin https://github.com/clojure/clojure/commit/53b02ef20c32386c4b4e23e1018e1a19140fee06

It’s hard to see how the performance was improved. How did it happen?

Oh .. I see that the new version has 1 less apply

I’ve also seen benefits of avoiding apply in babashka. I wrote a macro to optimize 20 arities for this

I’ve tested your simplified version and didn’t really see a performance difference compared to core

One thing I wonder about is why create an instance of up every time, instead of defing it as a private var? Wouldn't it save allocations and be able to be optimized by the JVM better?

@ben.sless Test it and you will know

Bringing up a REPL as we speak 🙂

now waiting for criterium to cook up an answer

huh

Execution time mean : 328.707164 ns
vs.
Execution time mean : 384.654215 ns

what's going on here? 😮