This page is not created by, affiliated with, or supported by Slack Technologies, Inc.
2020-08-06
Channels
- # announcements (4)
- # beginners (132)
- # calva (37)
- # chlorine-clover (60)
- # cider (1)
- # clara (12)
- # clj-kondo (40)
- # cljs-dev (109)
- # clojure (76)
- # clojure-dev (19)
- # clojure-europe (8)
- # clojure-france (17)
- # clojure-nl (4)
- # clojure-sg (1)
- # clojure-spec (14)
- # clojure-uk (7)
- # clojurescript (98)
- # conjure (96)
- # cursive (15)
- # data-science (2)
- # datalog (11)
- # datomic (24)
- # emacs (17)
- # figwheel-main (3)
- # fulcro (45)
- # jobs-discuss (1)
- # kaocha (3)
- # malli (2)
- # nrepl (1)
- # off-topic (135)
- # portal (2)
- # re-frame (17)
- # reagent (11)
- # reitit (4)
- # sci (60)
- # shadow-cljs (75)
- # spacemacs (3)
- # sql (32)
- # tools-deps (79)
- # vim (88)
- # xtdb (4)
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.
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 def
ing it as a private var? Wouldn't it save allocations and be able to be optimized by the JVM better?
@UK0810AQ2 Test it and you will know