re-use (done: false) IteratorResult objects yielded by underlying iterator

[michaelficarra]

Taken from the tests PR: https://github.com/tc39/test262/pull/4326/files#diff-667834d670268d2d741d9c1331df51d617cb4281fd9a46f785af4f43cc1ff256R49.

let oldIterResult = {
  done: false,
  value: 123,
};

let testIterator = {
  next() {
    return oldIterResult;
  }
};

let iterable = {
  [Symbol.iterator]() {
    return testIterator;
  }
};

let iterator = Iterator.concat(iterable);

let iterResult = iterator.next();

assert.sameValue(iterResult.done, false);
assert.sameValue(iterResult.value, 123);

assert.notSameValue(iterResult, oldIterResult);

This seems really wasteful. Would there be any harm in re-using these IteratorResult objects from the underlying iterator instead of constructing them anew?

[bakkot]

It's kind of weird to do this given that we don't do it for yield*, which is explicitly the "forward the result" operator.

I'm not totally opposed but it isn't obvious that this would be a perf improvement anyway, because now engines can't assume that the result of Iterator.concat is itself a well-behaved iterator result object. I have a doc about how engines could skip the allocation of these objects for built-in iterators in the common case of consuming the result as an iterable, but you wouldn't be able to do that optimization for Iterator.concat if we made this change.

[zloirock]

Iterator.prototype.{ take, drop, filter }

[michaelficarra]

Assuming we do this, we should also do this (as a follow-up) for yield* and Iterator.prototype.{take,drop,filter}. We shouldn't blindly follow their precedent here.

[anba]

It's kind of weird to do this given that we don't do it for yield*, which is explicitly the "forward the result" operator.

yield* forwards the iterator result object. https://tc39.es/ecma262/#sec-generator-function-definitions-runtime-semantics-evaluation:

i. Let innerResult be ? Call(iteratorRecord.[[NextMethod]], iteratorRecord.[[Iterator]], « received.[[Value]] »).
...
v. If done is true, then
...
vi. If generatorKind is async, set received to Completion(AsyncGeneratorYield(? IteratorValue(innerResult))).
vii. Else, set received to Completion(GeneratorYield(innerResult)).

It's maybe possible to change yield* to not forward the iterator result object, because JSC already has this behaviour: https://bugs.webkit.org/show_bug.cgi?id=174756.

[bakkot]

Ah jeeze, that's what I get for testing in a browser instead of actually reading the spec.

[michaelficarra]

I opened a needs-consensus PR for 262: tc39/ecma262#3489