Adds support for es6 iterators #898
Conversation
|
👍 The performance impact seems negligible. |
|
Probably should be tested in more environments before a decision is made
|
|
@megawac Maybe create an object {key, value} for a smaller penalty performance? function _iterator(coll) {
var i = -1;
var len;
var keys;
if (_isArrayLike(coll)) {
len = coll.length;
return function next() {
i++;
return i < len ? {value: coll[i], key: i} : null;
};
}
else if (iteratorSymbol && coll[iteratorSymbol]) {
var iterator = coll[iteratorSymbol]();
return function next() {
var item = iterator.next();
if (item.done) return null;
return _isArray(item.value) ? {value: item.value[0], key: item.value[1]} : {value: item.value};
};
}
else {
keys = _keys(coll);
len = keys.length;
return function next() {
i++;
return i < len ? {value: coll[keys[i]], key: keys[i]} : null;
};
}
}p.s. But not {0, 1}. This object will confuse V8 JIT optimizator. |
|
Afaik, arrays have historically been well optimized in v8
|
|
For iteration, but not for creation. |
|
Would you be interested in forking this branch and running the benchmarks?
|
|
I can do it little bit later (during this week). Why "return _isArray(item.value) ? item : [item]" is bad idea: var mySet = new Set();
mySet.add("a");
mySet.add({});
mySet.add([1, 2]);
var setIter = _iterator(mySet);
setIter(); // => ["a"];
setIter(); // => [{}];
setIter(); // => [1, 2], but we awaiting [[1, 2]]; |
|
How would you suggest plucking keys from a map iterator in that case? var m = new Map() m.set({a:1}, {b: 5}) mSymbol.iterator.next() On Mon, Oct 5, 2015 at 11:01 AM, Ilya [email protected] wrote:
|
|
Best solution would be Array.from but not all environments supports this. var m = new Map();
m.set({a:1}, {b:5});
m.set([1,2]);
_iterator(m);
....
else if (iteratorSymbol && coll[iteratorSymbol]) {
var iterator = Array.from(coll)); // Array.from(coll) == > [ [{a:1}, {b:5}], [[1,2], undefined] ]
len = iterator.length;
return function next() {
i++;
return i < len ? [iterator[i][0], iterator[i][1]] : null;
};
}
.... |
|
Wouldn't that run into the same issue with sets of arrays and arrays of On Mon, Oct 5, 2015 at 12:25 PM, Ilya [email protected] wrote:
|
|
Other cases of iterators return only value without key? Check for instanceof Map before calls Array.from? :) |
|
Probably makes most sense to ignore EntryIterator types (such as the map On Mon, Oct 5, 2015 at 12:59 PM, Ilya [email protected] wrote:
|
|
You mean a users must manually convert such objects to array/collection? |
|
Is there a simple way to detect the difference between the Map style iterator ( |
Leave
I don't believe so |
|
@megawac, sounds good. But still need some keys for forEachOf. Do you agree? else if (iteratorSymbol && coll[iteratorSymbol]) {
var iterator = coll[iteratorSymbol]();
return function next() {
i++;
var item = iterator.next();
if (item.done) return null;
return [item.value, i];
};
}async.forEachOf( ([value, key], index, next) => {}))@aearly, it's unreal. User can create custom iterator for his collections. |
|
I would suggest always giving On Tue, Oct 6, 2015 at 3:09 PM, Ilya [email protected] wrote:
|
|
Ok, i agree :) |
|
@megawac, async.map will return a empty array if keys will be 'undefined' |
|
Not if its implemented via _iterator
|
|
You suggest pass 'undefined' only in async.*Of, but _iterator must return keys always? |
|
One thing I discovered: creating simple Objects is slightly faster than creating Arrays: http://jsperf.com/object-vs-array-vs-class . Most JS engines convert small, non-dynamic Objects to structs internally. (I remember discussing this with @megawac a while back). This might make the cost of converting an interator that returns just values, to an iterator that returns {key, value} pairs. |
|
/cc @jdalton do you have any insight on ^ On Sun, Oct 25, 2015 at 9:05 PM, Alexander Early [email protected]
|
|
From the benchmarks it looks like perf is ok. As for the arrays or objects I think arrays map better to a map which can be initialized as an array of key-value pairs. I'm not sure what you're wanting to provide though as I see if you're iterating an array you return |
|
Now that we're using lodash internally, would we get iterator support for free if lodash supported them? |
Lodash has limited iterator support. It currently only supports them in |
|
Now that we've majorly refactored Async, this should be revisited in another PR. |
Still not sure if this is the right take to address this. The main issues is I expect there will be a perf hit as a result of creating a new array (
[value, key]) at each step iteration step, instead of just passing keys directly (benchmarks to follow).Fixes #839
Benchmarks