jlongster / transducers.js

A small library for generalized transformation of data (inspired by Clojure's transducers)
BSD 2-Clause "Simplified" License
1.72k stars 54 forks source link

Implement collection arity in terms of transducer arity? #5

Closed glenjamin closed 9 years ago

glenjamin commented 10 years ago

I managed to watch yesterday's strangeloop tranducers talk by Rich Hickey this morning: http://www.youtube.com/watch?v=6mTbuzafcII

One thing I noticed vs this lib was that functions which accept a collection just call sequence() on the transduce version of the function.

Was not doing this a design choice, or is it worth me sending a PR in?

jlongster commented 10 years ago

It's not like that because Clojure doesn't have to really worry about the performance of their transducers; a few functions calls in Java is nothing. But we have to worry about it, and JS is optimized for while loops and we'll never be able to beat that.

We need to make sure that map(fn, coll) is just as fast a while loop with an append inside it.

Right now there are 3 branches in each transformation: one for native array, one for a general collection, and one which returns a transducer (no coll supplied). The one for general collection could use the sequence form, yes. But I think instead we should make it more like the array while loop, since we can get an iterator from the collection. Although I think it's stupid, the JS world will compare the performance of map(fn, coll) between this and lodash on an array of 1000 items and if we just used sequence that would show very poorly on the benchmarks (even though I think it the perf of an 1000 array doesn't really matter).

All in all the code duplication within each transformation is bad and we can collapse it some, but I'd like to play around with way to keep it performant.

fson commented 10 years ago

@jlongster I think the if(isArray(coll)) branch is not really necessary in functions like map and filter, since the other branch will anyway delegate to reduce that implements the same check and optimized while loop for arrays.

Also the reducer function used with a collection is the same as the transducer, so I think map could be simplified to this (not tested):

function map(f, coll) {
  if (arguments.length === 2) {
    return reduce(coll, map(append), empty(coll));
  }
  return function(r) {
    var i = 0;
    return function(res, input) {
      return r(res, f(input, i++));
    }
  }
}
jlongster commented 9 years ago

I've greatly simplified this on master and it's way cleaner