ngAnimate 1.2.0-rc2 has performance impact

[damrbaby]

Just by including the ngAnimate module in my project (using latest code on master), I am seeing noticeable performance degradation that is especially visible on devices with slower processors like the iPod Touch 4g.

In my case an ngRepeat with around 50-100 items and no CSS transitions / animations takes what could be 2-3x longer to render on a mobile device with ngAnimate. I noticed that the performAnimation function is being called 50-100 times per digest (the number goes up or down depending on the number of items in the ngRepeat).

The animation code is being executed on all elements of directives that support animation whether or not there are actually CSS transitions / animations that apply to those elements, and it has a performance impact. For most animation-enabled directives it might not be a big deal but it is especially noticeable during the rendering and re-rending of items in an ngRepeat.

[mgcrea]

That's scary, we've also witnessed a few strange css regressions on some of our mobile applications, css transitions (fadeIn/fadeOut) with big pictures (retina) often get glitches on the latest rc2 but were working fine with v1.1.5, bug only happen on device (iPad), not as critical as this ngRepeat issue.

I quite liked the clean separation of animations and classes used in v1.1.5, it wasn't perfect, but imo it was less confusing & error/bug-prone that the way chosen for the v1.2.0. Moreover, I'm still wondering why you chose to release a release-candidate with such major breaking changes regarding animations, why not drop another 1.1.x release before the RC? I don't get the point of having an unstable branch to drop an such a clearly different (and somewhat broken) RC on the stable branch, it mislead people to think that animations were production-ready.

Anyway, if we have to add performance regression to the list, it's a little bit hard to digest. I think that mobile apps are the last territory to conquer for AngularJS, and performance will be a crucial stake. I'm having big hopes for AngularJS on mobile (and I pretty sure many developers feel the same), but theses animation issues are holding us back, it should really be the top 1 priority now.

[cburgdorf]

I'm also worried about performance of the new ngAnimate rewrite but I don't expect them to let this performance regressions slip into the final 1.2 release. I'm sure @matsko is on it and it will be resolved soon.

[JasonGoemaat]

It does seem rather crazy to do an animation cycle on elements when no animation is desired... ng-animate was a great way to control it..

[olostan]

Animations are one of the main features in upcoming 1.2 release. But as it used a lot on mobile devices, such performance drawback is critical...

Although I really liked the new style of doing animations via only css without modification of html/js...

[ajoslin]

One idea would be to have a 'strict' mode, where it only animates an element if ng-animate is present on the element.

myApp.config(function($animateProvider) {
  $animateProvider.onlyAnimateWithAttribute(true);
});

<div class="fade" ng-show="isShown" ng-animate>I will animate</div>
<div class="fade" ng-show="isShown">I won't animate</div>

[damrbaby]

@ajoslin I like your idea, and almost think that should be the default.

[ajoslin]

Yeah, that does seem to make more sense.

[mgcrea]

I'd second @ajoslin idea of a strict mode, would also default to it.

But if we have to add the attribute back, it may be worth considering reverting to use ngAnimate as an ngClass-like directive since it looks like the ngClass animation support has added quite some complexity & overhead to the directive compilation cycle (ie. https://github.com/angular/angular.js/commit/36ad40b18cfdd0690411a5169aa94e222946b5cf).

I'd also love to have some benchmarking tests (jsperf like) giving ranks to some basic use-cases, like render an ngRepeat with 1K elements, not sure if this could be easily achievable with jasmine/karma, but it would be great to have this kind of performance testing in the core.

[ajoslin]

I think it's being discussed; a benchmark could be done using e2e tests.

On Thursday, September 19, 2013, Olivier Louvignes wrote:

I'd second @ajoslin https://github.com/ajoslin idea of a strict mode.

I'd also love to have some benchmarking tests (jsperf like) giving ranks to some basic use-cases, like render an ngRepeat with 1K elements, not sure if this could be easily achievable with jasmine/karma, but it would be great to have this kind of performance testing in the core.

— Reply to this email directly or view it on GitHubhttps://github.com/angular/angular.js/issues/4011#issuecomment-24739551 .

...sent from my iPhone

[runxc1]

I have been developing a fairly heavy angularjs app with multiple instances of ngRepeat and a few different animations and I didn't notice any degradation of performance when upgrading from 1.1.5 to 1.2.0-rc1 but after updating to 1.2.0-rc2 I have noticed a significant performance hit and am probably going to need to roll back to 1.2.0-rc1. I have been able to get the app "performant enough" on desktop but it really starts to break down on an iPhone 4S where as it was working well with rc1.

[bfricka]

I really like this @ajoslin. ngAnimate on rc2 is no longer usable on mobile in the case of repeaters and all but the simplest cases. Additionally, there is still significant overhead generated just by including the module, even when setting $animate.enabled(false); due to all the timers still being registered and firing.

[joegaudet]

We also experience this regression, on tables of any considerable size. Seems like making animation inclusion declarative (as suggested above) is consistent with the angular ideals.

.joe

[matsko]

Hey guys. The solution isn't to tweak the template or to temporarily disable animations surrounding repeated lists, but to just improve the efficiency of $animate. The goal is that ngAnimate will be internally optimized to avoid calling performAnimation multiple times per digest and, as long as the className value is the same for the element as it was before and for each item in a list when compared to other items, then caching can boost the performance. This is the plan. Does anyone have a good benchmark I can look at to compare the caching code to?

[joegaudet]

The issue arrises for me when a repeater exists with a large number of rows, and then inside of the repeater there are visibility (hide / show) bits which all animate by default now. This all stacks up to make things really slow.

[olostan]

Did anybody achieved any results? Would appreciate any even temporary workaround (except downgrading version)

[joegaudet]

I just put a guard clause on perform animation for now - on my own fork. Works reasonably well.

[Narretz]

Any eta on this? Prevents me currently from testing the (bitter-)sweet new animations of 1.2, and all the other improvements

[matsko]

I've got a work in progress. Can anyone here use the JS files found here and test against their own code to see if things run any faster.

https://s3.amazonaws.com/angularjs-dev/faster-ng-repeat/angular.js https://s3.amazonaws.com/angularjs-dev/faster-ng-repeat/angular-animate.js https://s3.amazonaws.com/angularjs-dev/faster-ng-repeat/angular-route.js

[bfricka]

I'd be happy to do some testing with this. I likely won't be able to make time until this evening, but I will report back. On Oct 1, 2013 7:32 AM, "Matias Niemelä" notifications@github.com wrote:

I've got a work in progress. Can anyone here use the JS files found here:

https://s3.amazonaws.com/angularjs-dev/faster-ng-repeat/angular.js https://s3.amazonaws.com/angularjs-dev/faster-ng-repeat/angular-animate.js https://s3.amazonaws.com/angularjs-dev/faster-ng-repeat/angular-route.js

— Reply to this email directly or view it on GitHubhttps://github.com/angular/angular.js/issues/4011#issuecomment-25454309 .

[damrbaby]

@matsko It does not appear to be running any faster for me when rendering a new ng-repeat with 50+ items on a 4th generation iPod Touch (still takes what could be 4x longer than without ngAnimate). It doesn't look like the cache will help for a new ng-repeat, and brief debugging makes me believe the bulk of extra time is coming from the code in this forEach loop, which gets executed for each item in the ngRepeat list whether or not animations are present: https://github.com/matsko/angular.js/blob/1022d56669a8d52bce8e4dd66a9389e7b93a176b/src/ngAnimate/animate.js#L471-L475

[matsko]

@damrbaby that code is actually cached for the repeater, but I don't think that it causes any slowdown since the injection fetch is constant. Unless the try/catch brace around it slows things down. We'll find out after testing these files.

So I believe the bottleneck is coming mostly from the forced repaints that occur per each animation. A repaint (or reflow) occurs when there is a delay (timeout) during the JavaScript execution. Basically when the JS code gets a break from crunching synchronous code. You can also trigger a repaint using other tricks ... What ngAnimate does is it calls element.prop('clientWidth') each time enter/leave/move/addClass/removeClass is called to trigger the active animation and the reflow. This I believe really clogs the animation execution.

Here are the JS files which queue all the reflows into one operation, please let me know if the animation is any more performant. Instead of having N repaints you only have 1 repaint after N items have been processed.

https://s3.amazonaws.com/angularjs-dev/ng-animate-queue/angular.js https://s3.amazonaws.com/angularjs-dev/ng-animate-queue/angular-animate.js https://s3.amazonaws.com/angularjs-dev/ng-animate-queue/angular-route.js

[matsko]

@mzgol here's a commit for you: https://github.com/matsko/angular.js/commit/5e3fa50d931d8f84d573aa920ac85c66f278bb21

[damrbaby]

@matsko same issue, nothing has changed

[bfricka]

Really? No change? What metrics are you using @damrbaby ?

@matsko I have been getting my ass kicked at work but tomorrow I WILL test these changes against our main app and report back w/ some real numbers. Thanks for your time.

[mgol]

@matsko still lots of style recalculation.

[damrbaby]

ALRIGHT everybody I believe I've found the problem, sorry I didn't try and get more specific before.

The problem is when there's an ng-class in conjunction with an ng-repeat. The delay is slightly noticeable in Chrome but definitely noticeable on my 4th gen iPod touch.

This plunkr does not use ngAnimate, and takes less then 2 seconds to render on my iPod touch with 500 items in an ngRepeat (I tested this using plunkr's embedded view).

This plunkr DOES use ngAnimate, and takes 5+ seconds to render on my iPod touch also with 500 items in an ngRepeat.

The delay is also noticeable in chrome, I'm not kidding when I say it takes twice as long.

If you remove the ngClass from the ngRepeat, the delay goes away. So the issue is an ngClass + ngRepeat.

FYI both plunkrs are using @matsko's latest code posted above.

[bfricka]

@matsko

I was finally able to get this tested in our web app. The results are dramatic. I agree w/ @mzgol that there is still a lot of style recalculation going on but, the actual numbers are significantly better. On average the newer code is roughly 10x faster. This is the difference between a completely unusable layout and one where we might actually be able to use ngAnimate.

I wasn't able to get actual metrics for mobile (having problems w/ ADB extension), but it feels like a night and day difference. Before, ngAnimate would lock up mobile for up to 10 seconds. Right now it's about 1.5 seconds. Still not usable for a large mobile repeater. We're using lazy-loading, but it isn't very efficient right now as it's using a clean data set from a service which causes ngRepeat to remove and re-create all the elements each time. We might have to write a custom ngRepeat so we can make more effective use of caching and recycling elements and scopes. I think w/ this we might be able to hook into ngAnimate and actually have something usable. We'll see.

Either way, this is heading in the right direction, at least based on my testing.

[damrbaby]

I'd just like to add that any ng-show or ng-hide that appears within the ng-repeat slows things down incredibly as well with ngAnimate. I was able to get the 50-item repeater in my app to perform normally on my iPod touch with @matsko's latest code by removing all the ng-show's, ng-hide's, and ng-classes that were on and nested inside the ng-repeat.

[matsko]

Conveniently, there's another issue for animations similar to this and there's a commit which disables all child animations even if there is no animation present. This way you don't see all the child directives (ngShow, ngClass, etc...) attempt any animations during the remainder of the compilation and digest on the repeat/view/switch/include or if directive. So far it's working great on the docs page. Here's a link to the JS files:

https://s3.amazonaws.com/angularjs-dev/ng-animate-disable-children/angular.js https://s3.amazonaws.com/angularjs-dev/ng-animate-disable-children/angular-animate.js https://s3.amazonaws.com/angularjs-dev/ng-animate-disable-children/angular-route.js

Here's the SHA: https://github.com/matsko/angular.js/commit/de70d2a2a101ee8ab8db0a636fa0597203779192

Let me know if this helps out at all.

[damrbaby]

@matsko That's a big improvement I think it's almost as performant as without ngAnimate

I'd celebrate for joy, however, ironically that code does not work when things ARE supposed to animate. At least, it seems to work intermittently. For example, the sliding animations that I'm supposed to have in my app using ng-show and ng-if seem to only work once and then they stop working, whereas with the previous code even though things were incredibly slow to render initially they did work.

[matsko]

@damrbaby still ironing out the kinks. We're getting close :)

[matsko]

@damrbaby are you still experiencing bugs with these files?

https://s3.amazonaws.com/angularjs-dev/ng-animate-disable-fixed/angular.js https://s3.amazonaws.com/angularjs-dev/ng-animate-disable-fixed/angular-animate.js https://s3.amazonaws.com/angularjs-dev/ng-animate-disable-fixed/angular-route.js

[bfricka]

@matsko Using the code that you posted before (the one I tested against for my last response), we are seeing some issues w/ ngAnimate states not resolving correctly. Here's an example that occurs frequently, where ng-leave elements are not removed. We've also seen this with the ng-move state.

Using the files you posted above, ngAnimate no longer works at all for our repeater.

Let me know if you require additional details, or message me on Skype (bfricka)

[damrbaby]

@matsko latest code has same behavior as before, where most animations are no longer getting fired. It seems like the first animation in a particular view gets fired and then subsequent ones don't.

[the-machinist]

@matsko I see old views remaining within ng-view, like @brian-frichette's screenshot shows - and perhaps, as a result of that, events in the new scope/view don't work.

[matsko]

Yup there's a current issue with ng-leave not being caught. Working on it.

[mgol]

@matsko It seems excessive style recalculations are gone on matsko@b3dde55eb409480434399c8e4586e9bd4bb8f710. \o/ Great work! I'll happily re-test your patch once you fix those ng-leave bugs.

[matsko]

@damrbaby @brian-frichette @mzgol can any of you guys send me your code so I can debug it locally?

[bfricka]

Hi @matsko,

Unfortunately, the code is under NDA so I can't release it, and it is complex enough where I can't easily boil it down into something that does the same thing (at least not initially). I'll go ahead and debug the code myself and see if I can extract a generic enough scenario that can reproduce the bug, and assuming I can, I'll write that up and send it over.

[matsko]

OK in that event, please try these files. I think I found the bug that was causing the issue:

https://s3.amazonaws.com/angularjs-dev/resolved-ng-animate/build/angular.js https://s3.amazonaws.com/angularjs-dev/resolved-ng-animate/build/angular-animate.js https://s3.amazonaws.com/angularjs-dev/resolved-ng-animate/build/angular-route.js

[damrbaby]

@matsko that doesn't fix it for me... I'm going to go ahead and see if I can put together a plunkr that replicates the issue

[matsko]

@damrbaby @mzgol can we perhaps have a hangout or Skype session to resolve this in a faster manner?

[matsko]

@damrbaby keep in mind that the fix disables all child animations EVERY time a parent directive performs a DOM changing operation (ngInclude/ngSwitch/ngIf/ngRepeat/ngView). This may the reason why you're not seeing the animations. I explained it here in detail: https://github.com/angular/angular.js/issues/3215#issuecomment-25671979

[damrbaby]

@matsko feel free to hangout me mattwindwer

[damrbaby]

@matsko in my case the animations should be occurring on sibling elements. E.g. one slides into view from the right and the other slides out of view to the left. One is using ng-if to toggle visibility and the other ng-show. They are both children of a parent ui-view directive (ui-router) that doesn't have any animations applied. I'll be happy to give you a demo when you have a chance.

[the-machinist]

@matsko Here's what I use to test - https://github.com/the-machinist/angular-test/tree/master

In www/js/lib -

angular folder with version 1.2.rc2.x2 is your latest S3 upload. angular folder with version 1.2.rc2.x1 is the S3 upload just prior to that.

Sorry for the terrible example.. I kind of left it in between. Let me know if you need me to add anything else to it.

[damrbaby]

@matsko I've boiled the issue down to animations no longer firing after a location change. They work fine if I don't change the location and just use a local variable to toggle the animations on or off. I am not using the angular.js router but ui-router. The rc2 animation code worked, yet the last code doesn't. So the question is what changed in the latest code that would cause a location change to disable all future animations?

[damrbaby]

After lots of debugging it's still not clear to me what's going on.... basically the moral of the story seems to be that either ui-router needs to be updated or there is a bug in ngAnimate. Whatever the case may be things like ng-show and ng-if animated fine in rc2 with ui-router, but that came with a performance impact. With the latest code they animate once and then stop animating after a location change via $state.transitionTo ... even if that location change does not update the ui-view. I'd be curious if anybody else is using ui-router on this thread who might have some thoughts on this... other than that I can't say for sure if there is a bug at this point.

[mgol]

@matsko check your GitHub e-mail, please. :)

[matsko]

@damrbaby please email me so I can add you hangouts.