1342375 - Regression on misc-basic-array-forof by making IonBuilder::processIterators transitive

Status: NEW

(Core :: JavaScript Engine: JIT, defect, P3)

Description

[Hannes Verschore [:h4writer]]

See bug 1342369. This is about the first regression. Locally it changed score from 25 to 42 (ms).

Comment 1

[Hannes Verschore [:h4writer]]

This was backported to aurora. Marking as such.

Comment 2

[Shu-yu Guo [:shu]]

I'm not sure how to fix this regression. Bug 1331092 gets rid of the ITERCLOSE trynote, which the transitive processIterators is used for. After that lands, and we also move destructuring code off of trynotes, we can stop using processIterators for ES6-style iterators.

That said, IteratorClose is straight up slow. Bug 1331092 will implement them as try-catches, which I'm sure will have its own share of slowdowns. To improve here (which I don't have time to do in this bug because it'd be involved), we'd need to:

  1. TI knowing that we're iterating with well-behaved iterators that have no weird hooks like "return" or "throw". Builtins like arrays produce such iterators.
  2. Optimize out catch blocks for IteratorClose for such well-behaved iterators, and possible the calls to "next" as well.

Perhaps Ted will be interested? :)

Comment 3

[Shu-yu Guo [:shu]]

For extra clarity, IteratorClose is required by spec and we did not implement them before.

Comment 4

[Hannes Verschore [:h4writer]]

I'll give it P3. It seems that per spec we need to do some extra work that caused this regression. I'm still leaving this open, since we might want to search for a way to optimize this.