cujojs / when

A solid, fast Promises/A+ and when() implementation, plus other async goodies.
Other
3.44k stars 396 forks source link

IE8 performance issue #227

Closed jbadeau closed 10 years ago

jbadeau commented 10 years ago

Hi Brian,

We are experiencing extreme performance issues with when 2.6.0 (or any 2.x version).

I have created a repo: https://github.com/jbadeau/when-2.x-ie8-performance

which demonstrates the issue.

basically when 2.x is used the app leaks about 20mb on refresh and load time is heavily impacted. IE8 becomes completely unusable in a moderately complex wire app in about 3 refreshes.

I hate having to support IE8 but I cant get rid of it yet :(

This is a very critical issue for us. If you need any additional info or help please let me know.

Cheers, Jose

briancavalier commented 10 years ago

@jbadeau Thanks for the test case. One major difference in when 1.x and 2.x is that 2.x, in order to be Promises/A+ compliant, must resolve promises asynchronously. In IE8, when.js 2.6 falls back to setTimeout, but also uses a queue processing algorithm that minimizes the number of setTimeout calls. There will definitely be some small amount of slowdown due to that, but I definitely wouldn't expect it to be leaking.

I tried your example in a few other browsers and it doesn't seem to leak. The memory increases slightly on page loads, but then is reclaimed (when the garbage collector runs, presumably) periodically … over time, it looks quite stable in Chrome, FF, and Safari. I'll fire up an IE VM and take a look.

What are you using to measure the memory? Are you just looking at windows task manager?

jbadeau commented 10 years ago

Hi Brian,

This is only an issue with IE8, all other browser including IE9+ work fine. I am using "Process Explorer" to watch memory.

briancavalier commented 10 years ago

@jbadeau Cool. thanks for the additional info. This sounds like it'll be a fun one to debug, heh :/

jbadeau commented 10 years ago

Yeah, I had a quick look with the IE8 profiler but wasn't too useful for me. Im gonna have a look now with DynaTrace

briancavalier commented 10 years ago

Awesome, any additional info is certainly welcome :)

jbadeau commented 10 years ago

An update:

So I have removed the memory leak. This is a leak in dojo with their console shim. However the load time issue is still there.

I have seen that the memory consumption is very high (up to 3x) on a refresh. It looks like allot of garbage collection is happening.

The load issue only seems to happen in the runHandlers method (I commented it out).

I have tried setting the handlerQueue to null on unload but no change. I have also increased the setTimeout to 3 seconds but no change.

I'll have another go at it today!

briancavalier commented 10 years ago

Interesting. Good to know about the dojo console shim (does it still use Firebug Lite?) I did a little testing as well, and my current theory is that this is probably due to the fact that wire tends to use a very large number of promises during wiring. So, when the app is starting up (i.e. page is loading, wire is wiring some initial specs), it is generating lots of short-lived promises that then have to be garbage collected.

A couple quick questions:

  1. When you say 3x memory consumption, what exactly do you mean? For example, 3 x ?
  2. Do you see the memory usage go back down after the 3x memory spike?

And here's something you can try. I created this issue-227 branch where we can do some experiments. It has a small change that keeps the handlerQueue from growing as much. Could you try it out and let me know what effects it has. I don't expect this to be a "fix", per se, but I'm hoping we learn something from seeing how it behaves compared to 2.6.0.

Thanks for working through this!

jbadeau commented 10 years ago

The dojo code in question is from the dojo/_base/kernel module. It basically fills in missing firebug console methods

"assert", "count", "debug", "dir", "dirxml", "error", "group", "groupEnd", "info", "profile", "profileEnd", "time", "timeEnd", "trace", "warn", "log"

with noop methdos so that code doesnt fail when no firebug or compatible console is installed. They still use firebug light for old browsers that dont have a console (eg ie6 ie7).

Q1. I mean that after the inital load the memory consumption could be 80mb, on referesh the memory usuage jumps to 240. At 240 the garbage collection kicks in and memory may drop to 60mb and then back to 240 untill the page loads. After the memory spikes, the memory returns to original 60mb.

I have also seen that there are allot of promises with wire but when 1.X handles this with no problem.

What is the behaivor if the queue cannot be trained in one tick? Are the handlers invoked again even if resolved?

Another thing; is it possible to have more than 1 concurrent setTimeout (drain takes longer than 0ms)? many setTimeouts have issues with memory? Perhaps using 1 setTimeout and clearing the curent one or using setInterval may help. I will try this out?

briancavalier commented 10 years ago

Ah, ok, so at least we know it is getting garbage collected. In 1.x, resolutions were not forced into a future turn, so the handler queue could be unwound in the current call stack. In 2.x, the handlers have to be queued until the stack clears (this is a Promises/A+ requirement), which means that the handler queue typically grows larger and lives longer in 2.x than 1.x. Or, to put it another way, because when.js 1.x violated the "wait until the stack clears to process handlers" requirement of Promises/A+, it could clear the queue more often--thus it never grew as large.

That all aligns with the theory that the large number of promises created by wire causes the spike.

Is the 240mb spike acceptable for your app? Or is it causing a problem (like crashing your app in IE!)?

I'd certainly love to reduce the memory usage, and I think it would take 2 things:

  1. Reduce when.js's overall memory usage. There are probably some small things that can be done, and
  2. Try to reduce the number of promises that wire creates. There's probably more that can be done here.

I'll create issues for those.

jbadeau commented 10 years ago

So the behavior of branch issue-227 vs 2.6.0 is:

2.6.0

227:

jbadeau commented 10 years ago

The problemis very severe for us. refershes go from 4sec inital load times to 2 minutes plus the ui is unusable.

briancavalier commented 10 years ago

@jbadeau when 2.7.1 has some perf improvements and reduces overall memory usage. If you have a chance, could you try it and let me know if it helps your situation (even if it doesn't completely solve the problems you're seeing). Thanks!

jbadeau commented 10 years ago

i feel the need.................the need for speeeeeeed :P

jbadeau commented 10 years ago

Will get back to to you with the results

jbadeau commented 10 years ago

Hi Brian, unfortunately there seems to be no impact on IE performance. I think the main problem is the amount of promises wire generates. We now conditionally load when depending the client. :) Thanx for the help too.

stefanpenner commented 10 years ago

@jbadeau i am curious how many promises you allocate, and if it is possible for me to view a demo of this.

briancavalier commented 10 years ago

@jbadeau when 3.0 reduces the memory usage drastically. It typically uses half or less the memory of 2.x. It's still in a branch under development, but is fully backward compatible. I haven't tried it with wire yet, but I'll see if I can give it go today.

jescalan commented 10 years ago

Pretty pumped about 3.0. Do you have a rough estimate on when it will be ready to use? If not that's fine, just asking :grinning:

briancavalier commented 10 years ago

@jenius Cool, it's getting close. "Sometime in February" is probably a good estimate :) There's some code cleanup to do, plus finishing unit test coverage and writing docs for new APIs.

jbadeau commented 10 years ago

By the way Brian, the itop platform (ebanking asia) is ready for production. Unfortunate you must be a ubs asia client to access the platform. However there is a movie about it at http://www.ubs.com/hk/en/mediaportal/itop.html The images are from the platform using mostly cujojs, knockout, bootstrap and custom mvc framework. So thanx for all the sweet libs. The relevant section of the video is the "ebanking platform"

jescalan commented 10 years ago

Awesome, looking forward. Let me know if there's any way I could help out!

briancavalier commented 10 years ago

@jbadeau That's awesome news. Congrats! And thanks for the pointer to the movie, I'll check it out in a bit.

unscriptable commented 10 years ago

Congrats @jbadeau! Which parts of cujojs are you using? Can we tweet about itop?

jbadeau commented 10 years ago

I can give u a marketing summary next week if u want? We are using wire, poly, when, meld, rest and msgs.

briancavalier commented 10 years ago

I can give u a marketing summary next week if u want?

Please do! That would be great. Are you thinking email, or something more real-time?

We are using wire, poly, when, meld, rest and msgs.

Awesome. :+1: Pinging @scothis, as he deserves the rest and msgs kudos.

briancavalier commented 10 years ago

@jenius Are you up for creating a few unit tests for the new algebraic/monad APIs?

jescalan commented 10 years ago

I can definitely give it a shot. Any specific pointers before I start?

jbadeau commented 10 years ago

I will have a check if there is some official stuff I can send to you guys.

jbadeau commented 10 years ago

A colleague is checking if UBS can officially endorse that we use cujojs libs. Might take a few days (big banks for u). Ill try to write up a small blurb about our experience with cujojs

briancavalier commented 10 years ago

Thanks, @jbadeau, really appreciate your looking into it. That would really be great news if it's possible. If you need anything from us, just let us know!

cc @unscriptable

briancavalier commented 10 years ago

@jbadeau when 3.0 was just released. It's a pretty massive perf boost (10-20x) and memory reduction over 2.x. Wire 0.10 doesn't quite work with it yet due to some API changes, but I am working on a wire update release that will add when 3.0 compatibility. Hopefully it will help your use cases!

jbadeau commented 10 years ago

Hi Brian, Will drop in 3.x and see how it goes. Will report back with the results. On another note, thank you very much for the when.settle method. Was just what I was looking for the other day.

briancavalier commented 10 years ago

Cool. Keep an eye on cujojs/wire#157 where we're working on updating wire to be compatible with when 3.x. Targeting next week.

Glad to hear that when.settle was helpful! Any chance you can describes pursue case or the problem it helped you solve?

jbadeau commented 10 years ago

I needed the when.settle to:

I am developing a JS osgi container on start of the container the bundles are installed asynchronously using promises. If a bundle cannot install then it rejects its promise; however I want to be able to log all bundle install results success or fail. When.all and most helper methods only returns the first reject results.

briancavalier commented 10 years ago

Nice, that's a perfect use case. Thanks!

briancavalier commented 10 years ago

Hey @jbadeau, any word on latest performance in IE using when 3.x?

jbadeau commented 10 years ago

Hey Brian. Unfortunately we went live with the 2.x branch and were not able to test the 3.x. We will move to 3.x but prob not till 4th quarter.

briancavalier commented 10 years ago

Ah, ok, no worries. I was just curious :) When you get to that point, feel free to ping me, as always.

In the meantime, what would you like to do with this issue? One possibility is that we close it, and you can create a new one if you have a specific issue in production with 2.x. I'm ok with however you'd like to handle it, though.

jbadeau commented 10 years ago

I agree. It can be closed. We have a solution for IE8 so I don't see any need for this issue anymore :)

briancavalier commented 10 years ago

Thanks!