<< Back to previous view

[CLJS-760] Add an IAtom protocol Created: 01/Feb/14  Updated: 17/Feb/14  Resolved: 17/Feb/14

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Jamie Brandon Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-760.patch     Text File CLJS-760.patch     File patch    
Patch: Code

 Description   

Add an IAtom protocol with a -reset! method and a fast path for Atom in cljs.core/reset!.

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

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



 Comments   
Comment by David Nolen [ 02/Feb/14 11:40 AM ]

This is not a properly formatted patch - http://github.com/clojure/clojurescript/wiki/Patches.

Comment by Jamie Brandon [ 02/Feb/14 11:51 AM ]

Fixed

Comment by David Nolen [ 02/Feb/14 6:33 PM ]

fixed https://github.com/clojure/clojurescript/commit/33692b79a114faf4bedc6d9ab38d25ce6ea4b295

Comment by Jozef Wagner [ 03/Feb/14 2:40 AM ]

What is the rationale behind this? Do you plan to have multiple Atom types?

Comment by David Nolen [ 03/Feb/14 8:13 AM ]

There's no good reason to lock down atom behavior to the one provided by ClojureScript.

Comment by Jozef Wagner [ 03/Feb/14 10:25 AM ]

I agree that such behavior should be polymorphic, I was just surprised that it is a pressing issue in a single threaded CLJS. Clojure itself does not provide such abstraction. Anyway, if you want to get it right, I suggest also to include swap! and compare-and-set! in the protocol (in order to not lock down the sharing and change policy), and to name it something like IMutableReference, as these methods are generic enough to be used outside atom, (as atom promises additional features besides those of a simple mutable reference). See https://gist.github.com/wagjo/8786305 for inspiration.

Comment by David Nolen [ 03/Feb/14 11:21 AM ]

I question the utility of compare-and-set! given most JS hosts are single threaded (but who knows? things are changing quickly in the JS world). It's something to consider in the future. IMutableReference is just more to type and I'm not really convinced it offers anything semantically over IAtom. Atoms do support more functionality but this is covered in other protocols.

Comment by David Nolen [ 03/Feb/14 11:21 AM ]

We need to include -swap! in the IAtom protocol.

Comment by Jozef Wagner [ 03/Feb/14 12:02 PM ]

The usefulness of the atom and protocols behind it should 'transcendent' the notion of single/multiple threads. Prime example is the Avout and its zk-atom and zk-ref. If e.g. implementing Avout in CLJS is a reasonable and useful thing to do, we should prepare for it by designing protocols to handle such use cases.

Comment by David Nolen [ 03/Feb/14 12:30 PM ]

Avout is an third party library which has no relation to the direction of ClojureScript. This ticket is a conservative change that opens up the door for extending atom-like capabilities to user types, that's it. Further enhancements may or may not come in the future.

Comment by David Nolen [ 17/Feb/14 1:49 PM ]

fixed https://github.com/clojure/clojurescript/commit/3e6564a72dc5e175fc65c9767364d05af34e4968

Generated at Tue Sep 02 03:00:38 CDT 2014 using JIRA 4.4#649-r158309.