<< Back to previous view

[CLJS-508] Missing IReduce implementations and typo in clojure.core.reducers/append! Created: 21/May/13  Updated: 27/Jul/13  Resolved: 21/May/13

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

Type: Defect Priority: Minor
Reporter: Daniel Skarda Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None
Environment:

Fresh CLJS checkout from github


Attachments: Text File 0001-Add-IReduce-protocol-to-LazySeq.patch     Text File reducers.patch    
Patch: Code and Test

 Description   

1) IReduce protocol is missing implementation for array and List.
2) append! uses (.add array), while the correct array method name is push

Patch attached.

Note: what is your experience with array.push performance? ClojureScript reducers/foldcat uses arrays for append! / cat implementation. My experience was that (into [] ...) using conj! and TransientVector was at least 5 times faster than reducers/foldcat using array and push. The reason is probably inefficient push implementation in Chrome/V8



 Comments   
Comment by David Nolen [ 21/May/13 9:59 AM ]

Thanks for the patch. I doubt that array push is inefficient, but I imagine the regularity of TransientVector operations is something that V8 likes.

Comment by David Nolen [ 21/May/13 10:03 AM ]

fixed, http://github.com/clojure/clojurescript/commit/30bb0b9e55fb77cdfe952fbf5df763a25c4a25c5

Comment by David Nolen [ 22/May/13 11:36 PM ]

BTW I notice you haven't submitted your Contributor Agreement (unless I'm mistaken). Please send that in at your earliest convenience thanks much!

Comment by Daniel Skarda [ 23/May/13 3:02 AM ]

Patch for missing IReduce protocol for LazySeq

Comment by Daniel Skarda [ 23/May/13 3:10 AM ]

@swannodette: Ok, I will send it today.

Comment by David Nolen [ 27/May/13 3:52 PM ]

Many thanks!

Generated at Thu Nov 20 17:13:23 CST 2014 using JIRA 4.4#649-r158309.