1317690 - Support Generators in IonMonkey

Status: NEW

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

Description

[Till Schneidereit [:till]]

With async/await, Generator performance becomes more important, so at some point we should support them in IonMonkey.

I don't know *how* hard this is, just that it is hard. I also don't really know the right priority for this: the public web won't be using native async/await or Generators for some time, but platform code and addons certainly will.

Comment 1

[Jan de Mooij [:jandem]]

It would be interesting to check how we actually perform here, compared to other engines. Our generators were a bit faster than in V8 when I added Baseline support, but Turbofan is supposed to handle generators so that probably changed.

Comment 3

[johan]

Hi,

We are using a generator heavy approach in a new web component. Turns out it runs really slowly in Firefox. Found this jsperf (https://jsperf.com/generator-overhead-2) and tried it on Chrome, Safari and FF on my machine. Safari was roughly twice as fast as FF, Chrome seven times faster. Any hope of this improving soon? Would be great!

Regards,
Johan Isaksson

Comment 4

[Nickolay Platonov]

Any hope for this issue to be resolved? Is it on the radar at least (included in the roadmap)?

Comment 5

[Nicolas B. Pierron [:nbp]]

(In reply to Nickolay Platonov from comment #4)

Any hope for this issue to be resolved? Is it on the radar at least (included in the roadmap)?

From what I recall this was in the pipeline for December 2019, however, we since decided to simplify a lot SpiderMonkey to reduce the security risks related to IonBuilder usage. There is a project which is currently investigating the simplification of IonBuilder, and maybe replace it by WarpBuilder (Bug 1613592).

While this is not directly related, this is a higher priority at the moment.
I will raise this topic to the team, to see if we should raise the priority of generators in IonMonkey.

Comment 6

[Nickolay Platonov]

Thanks for the update! Yes, please draw some attention to this issue. We wrote a "modern" system, that uses generators a lot, and discovered the performance in FF is very bad (#1565549). In the same time its so powerful feature, arguably a best addition to JS spec in its history.

Comment 7

[Jan de Mooij [:jandem]]

Yeah we expect this to be significantly easier with WarpBuilder in the future. Note though that Ion/Warp-compiling them is just part of it, generators also require more environment objects right now, and that could be a bigger issue than them not running in Ion.

Comment 9

[johan]

Hi,

Any hope of generators getting optimized any time soon? We have for the last 5 years had to recommend our customers to pick another browser when using our components, ideally they should be able to use Firefox too.

Comment 10

[Mats]

Adding my voice here too, this is becoming a critical defect for us. Seems odd that FF is so far behind Chrome and the rest for 8+ years.

Do you offer a way for us sponsor a fast-tracking this implementation?

Comment 11

[Jan de Mooij [:jandem]]

Is there a public website we could use to reproduce the performance problem you're seeing? If not, maybe you could share a performance profile from the Firefox profiler?

Comment 12

[Nickolay Platonov]

See a benchmark here for a example: https://bugzilla.mozilla.org/show_bug.cgi?id=1565549

Comment 13

[Mats]

@Jan de Mooij - Any idea if we could somehow sponsor the fast-track of this bug?

Comment 14

[Jan de Mooij [:jandem]]

(In reply to Mats from comment #13)

@Jan de Mooij - Any idea if we could somehow sponsor the fast-track of this bug?

At this point it would be most useful for us to have access to a website that demonstrates the performance problem. That would help us determine if there's low-hanging fruit, other performance issues, parts we could prioritize, etc.

Comment 15

[Mats]

Isn't this test case enough? https://bugzilla.mozilla.org/show_bug.cgi?id=1565549

Comment 16

[Jan de Mooij [:jandem]]

(In reply to Mats from comment #15)

Isn't this test case enough? https://bugzilla.mozilla.org/show_bug.cgi?id=1565549

The code that's being used on an actual website is likely pretty different from the micro-benchmark linked there. There are multiple parts to optimizing generators and we just want to make sure we're looking at a representative benchmark, to avoid optimizing something that isn't relevant for your framework.

Comment 17

[Nickolay Platonov]

To examine things on real website, please do the following:

1. Open the https://bryntum.com/products/gantt/examples/bigdataset/ page. (If you'd like to check other examples, see https://bryntum.com/products/gantt/examples/)
2. In console, type: window.DEBUG = true
3. Click the "5K Tasks" button in the top toolbar. This will switch the example gantt to a new dataset of 5K tasks.

You should see the following output in console:

For Chrome:

window.DEBUG = true
true
generate: 25.85107421875 ms
Initializing project
Populating project: 887.9541015625 ms
Time to visible: 5474.97509765625 ms
Finalize propagation: 708.40185546875 ms

For Firefox:

window.DEBUG = true
true
generate: 303ms - timer ended
Initializing project
Populating project: 18714ms - timer ended 
Time to visible: 168067ms - timer ended 
Finalize propagation: 21519ms - timer ended

What happens here is:

1. generate - the example data is generated. Generation uses generators, perhaps that explains the 10x slowdown compared to Chrome, however, this is not a primary metric we are interested in.
2. Populating project - example data is loaded into "reactive graph". This does not involve generators, but instantiates lots of small objects (like hundreds of thousands of them). 21x slowdown compared to Chrome, would be great to see it optimized to be comparable.
3. Time to visible - this metric is the topic of this issue. Here the "reactive graph" calculates the final state. 30x slowdown compared to Chrome. Calculations are generators-based. During calculations, it also allocates the edges in the graph (populates lots of Map instances).
4. Finalize propagation - not related to generators, 30x slowdown.

Hopefully the numbers are self-explanatory, if you need any other information and/or benchmark, please let me know.

Probably one need to optimize the allocation speed too, because the most important metric Time to visible is a combination of generator calls + allocation. In particular, the allocation of Maps, and internal allocations by Maps. I'll open a new ticket for that.

Comment 18

[Mayank Bansal]

Just opening https://bryntum.com/products/gantt/examples/bigdataset/
Nightly: https://share.firefox.dev/4b9raNf (3s)
Chrome: https://share.firefox.dev/4bb4Umm (1.8s)

Clicking on 5K
Nightly: https://share.firefox.dev/3UQdzVo (12s)
Chrome: https://share.firefox.dev/3WPzMVd (3.5s)

---

With window.DEBUG = true , I generally get 11s with Firefox. But one time, Firefox took 45s.

Comment 19

[Nickolay Platonov]

Separate issue to optimize the allocations: https://bugzilla.mozilla.org/show_bug.cgi?id=1895667

Comment 20

[Jan de Mooij [:jandem]]

Thanks, that's helpful.

There's some generator overhead, but it's mostly time spent elsewhere. There are slow GetProp ICs that might benefit from bug 1878158. These are mostly looking up fieldMap and getFieldDefinition properties. There's also some Map overhead (bug 1851662).

Comment 21

[Jan de Mooij [:jandem]]

(In reply to Jan de Mooij [:jandem] from comment #20)

There are slow GetProp ICs that might benefit from bug 1878158. These are mostly looking up fieldMap and getFieldDefinition properties. There's also some Map overhead (bug 1851662).

To expand on this a bit: the JS code has a lot of JS getter functions. If you could make these non-getter properties it would likely make things faster in all browsers.

Comment 22

[Nickolay Platonov]

Thanks for pointing, we'll try to improve in this regard.

Comment 23

[Mats]

@Jan de Mooij: Just checking in, is this being worked on by anyone? We are facing massive issues in our business due to this issue (and related perf issues above)

Comment 24

[Matthew Gaudet (he/him) [:mgaudet]]

So, this isn't being actively looked at at the moment. But what I am going to do is spin the Bryntum specific aspect of this bug out into another bug. This is the wrong bug to be having all this discussion.

Comment 25

[Matthew Gaudet (he/him) [:mgaudet]]

I've opened Bug 1898545; we should move discussion of Bryntum specifically there.

Comment 26

[BugBot [:suhaib / :marco/ :calixte]]

Clear a needinfo that is pending on an inactive user.

Inactive users most likely will not respond; if the missing information is essential and cannot be collected another way, the bug maybe should be closed as INCOMPLETE.

For more information, please visit BugBot documentation.

Comment 27

[Mats]

Any updates on this? Do you have an ETA when Generators will be properly supported?

Comment 28

[Matthew Gaudet (he/him) [:mgaudet]]

No update; in the hierarchy of things to work on unfortunately this still is quite low.

Comment 29

[Mats]

Ok that's unfortunate. Any option for us to pay/sponsor this issue to increase priority?

Comment 30

[Iain Ireland [:iain]]

Based on the profiles in bug 1898545, I think generators are not the primary cause of your performance issues.