Why ClojureScript atoms do not implement the full protocol?

Question

swap! function swap! , one of the most idiomatic tools in the Clojure toolbar, does instance? validation instance? . We are told in programming to avoid applying conventions around type checking in order to prefer polymorphism (protocols). It seems strange that ClojureScript does not implement the ISwap protocol directly against atoms and instead in a public swap! The api returns to the protocol only after checking whether the object is an atom.

I assume that this tactic should have been used for performance reasons, as atoms are the main use case for swap! and many other atomic methods. Is it correct?

I would prefer to implement the api atom as part of the actual protocol so that this is unnecessary.

 (defn swap! "Atomically swaps the value of atom to be: (apply f current-value-of-atom args). Note that f may be called multiple times, and thus should be free of side effects. Returns the value that was swapped in." ([af] (if (instance? Atom a) (reset! a (f (.-state a))) (-swap! af))) ([afx] (if (instance? Atom a) (reset! a (f (.-state a) x)) (-swap! afx))) ([afxy] (if (instance? Atom a) (reset! a (f (.-state a) xy)) (-swap! afxy))) ([afxy & more] (if (instance? Atom a) (reset! a (apply f (.-state a) xy more)) (-swap! afxy more)))) 

Answer 1

This seems to be related to performance: http://dev.clojure.org/jira/browse/CLJS-760

Add the IAtom protocol with - reset! method and quick path for Atom in cljs.core / reset !.

See jsperf here - http://jsperf.com/iatom-adv

Recent versions of chrome and firefox suffer ~ 20-30% slowdown. Older versions of firefox suffer up to 60-70%.

Further on the tickets, it was decided to break IAtom into two protocols: IReset and ISwap. But that was the implementation that David passed with, who checks the type, and I believe that this was done to speed things up.

Unfortunately, it is unclear why Atom was not created to implement IReset (and ISwap), and why these things were not looked for. And it's unclear how the original patch works. He basically performed the reset! implementation reset! and put it under the instance check and added the -reset! path for it -reset! :

 diff --git a/src/cljs/cljs/core.cljs b/src/cljs/cljs/core.cljs index 9fed929..c6e41ab 100644 --- a/src/cljs/cljs/core.cljs +++ b/src/cljs/cljs/core.cljs @@ -7039,6 +7039,9 @@ reduces them without incurring seq initialization" ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; Reference Types ;;;;;;;;;;;;;;;; +(defprotocol IAtom + (-reset! [o new-value])) + (deftype Atom [state meta validator watches] IEquiv (-equiv [o other] (identical? o other)) @@ -7088,14 +7091,16 @@ reduces them without incurring seq initialization" "Sets the value of atom to newval without regard for the current value. Returns newval." [a new-value] + (if (instance? Atom a) (let [validate (.-validator a)] (when-not (nil? validate) - (assert (validate new-value) "Validator rejected reference state"))) + (assert (validate new-value) "Validator rejected reference state")) (let [old-value (.-state a)] (set! (.-state a) new-value) (when-not (nil? (.-watches a)) - (-notify-watches a old-value new-value))) - new-value) + (-notify-watches a old-value new-value)) + new-value)) + (-reset! a new-value))) (defn swap! "Atomically swaps the value of atom to be: 

This was done at 33692b79a114faf4bedc6d9ab38d25ce6ea4b295 (or at least something very close to it). And then other protocol changes were made in 3e6564a72dc5e175fc65c9767364d05af34e4968:

 commit 3e6564a72dc5e175fc65c9767364d05af34e4968 Author: David Nolen < [e-mail] > Date: Mon Feb 17 14:46:10 2014 -0500 CLJS-760: break apart IAtom protocol into IReset & ISwap diff --git a/src/cljs/cljs/core.cljs b/src/cljs/cljs/core.cljs index 25858084..e4df4420 100644 --- a/src/cljs/cljs/core.cljs +++ b/src/cljs/cljs/core.cljs @@ -7061,9 +7061,12 @@ reduces them without incurring seq initialization" ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; Reference Types ;;;;;;;;;;;;;;;; -(defprotocol IAtom +(defprotocol IReset (-reset! [o new-value])) +(defprotocol ISwap + (-swap! [of] [ofa] [ofab] [ofab xs])) + (deftype Atom [state meta validator watches] IEquiv (-equiv [o other] (identical? o other)) @@ -7124,21 +7127,33 @@ reduces them without incurring seq initialization" new-value)) (-reset! a new-value))) +;; generic to all refs +;; (but currently hard-coded to atom!) +(defn deref + [o] + (-deref o)) + (defn swap! "Atomically swaps the value of atom to be: (apply f current-value-of-atom args). Note that f may be called multiple times, and thus should be free of side effects. Returns the value that was swapped in." ([af] - (reset! a (f (.-state a)))) + (if (instance? Atom a) + (reset! a (f (.-state a))) + (-swap! a (deref a)))) ([afx] - (reset! a (f (.-state a) x))) + (if (instance? Atom a) + (reset! a (f (.-state a) x)) + (-swap! a (f (deref a) x)))) ([afxy] - (reset! a (f (.-state a) xy))) - ([afxyz] - (reset! a (f (.-state a) xyz))) - ([afxyz & more] - (reset! a (apply f (.-state a) xyz more)))) + (if (instance? Atom a) + (reset! a (f (.-state a) xy)) + (-swap! a (f (deref a) xy)))) + ([afxy & more] + (if (instance? Atom a) + (reset! a (apply f (.-state a) xy more)) + (-swap! a (f (deref a) xy more))))) (defn compare-and-set! "Atomically sets the value of atom to newval if and only if the @@ -7149,13 +7164,6 @@ reduces them without incurring seq initialization" (do (reset! a newval) true) false)) -;; generic to all refs -;; (but currently hard-coded to atom!) - -(defn deref - [o] - (-deref o)) - (defn set-validator! "Sets the validator-fn for an atom. validator-fn must be nil or a side-effect-free fn of one argument, which will be passed the intended 

This does not mean that the ticket is twofold: performance is a problem (although it’s not clear how: “atoms do not work fast enough”, or “other things using reset!” Do not work fast enough “?) And the design problem (“ we want the IAtom protocol "). I think the problem was that other implementations would have to transfer validation and notify the costs of the observers, even if they are not atoms. I would like this to be clearer.

One thing that I didn't like about commits in Clojure / Script is that they are not very descriptive. I would like them to be more similar to the core with the corresponding initial information, so that people (for example, we) would try to find out how everything turned out, there would be more useful information about this.