Repository navigation
ngAnimate 1.2.0-rc2 has performance impact #4011
Description
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.
Activity
mgcrea commentedon Sep 16, 2013
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
ngRepeatissue.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 thev1.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 commentedon Sep 16, 2013
I'm also worried about performance of the new
ngAnimaterewrite 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 commentedon Sep 17, 2013
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 commentedon Sep 18, 2013
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 commentedon Sep 19, 2013
One idea would be to have a 'strict' mode, where it only animates an element if
ng-animateis present on the element.damrbaby commentedon Sep 19, 2013
@ajoslin I like your idea, and almost think that should be the default.
ajoslin commentedon Sep 19, 2013
Yeah, that does seem to make more sense.
mgcrea commentedon Sep 19, 2013
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
ngAnimateas anngClass-like directive since it looks like thengClassanimation support has added quite some complexity & overhead to the directive compilation cycle (ie. 36ad40b).I'd also love to have some benchmarking tests (jsperf like) giving ranks to some basic use-cases, like render an
ngRepeatwith 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 commentedon Sep 19, 2013
I think it's being discussed; a benchmark could be done using e2e tests.
On Thursday, September 19, 2013, Olivier Louvignes wrote:
...sent from my iPhone
runxc1 commentedon Sep 19, 2013
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 commentedon Sep 20, 2013
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 commentedon Sep 20, 2013
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 commentedon Sep 20, 2013
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
performAnimationmultiple 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 commentedon Sep 20, 2013
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 commentedon Sep 20, 2013
Did anybody achieved any results? Would appreciate any even temporary workaround (except downgrading version)
65 remaining items
joegaudet commentedon Oct 10, 2013
Oh yes and a big thanks !
On Thu, Oct 10, 2013 at 9:04 AM, Joe Gaudet joe@joegaudet.com wrote:
.joe out.
+1.778.994.4846
matsko commentedon Oct 10, 2013
Should be in master tomorrow.
Narretz commentedon Oct 10, 2013
master is not in bower, though. This'll land in 1.2 RC3, I think.
mbelshe commentedon Oct 10, 2013
@matsko - Just wanted to chime in and let you know that your test files definitely fixed my ngAnimate performance problem as well. Look forward to it landing! Thanks!
fix($animate): perform internal caching on getComputedStyle to boost …
matsko commentedon Oct 11, 2013
Happily landed as ddcbf76
guillaume86 commentedon Nov 4, 2013
@matsko could you specify what is exactly expected in css/dom to trigger animations with the last changes?
I have trouble restoring animations in this case (angular animate the entering/leaving, masonry the positionning):
1.1.5: http://jsfiddle.net/g/3SH7a/
1.2.0-rc3: http://jsfiddle.net/g/3SH7a/45/
matsko commentedon Nov 4, 2013
RC3 introduces a restriction on placing the transition on the base class. However, with RC4 (which is 1.2.0) this restriction is gone. So wait a few days until that's out to test your code against that.
fix($animate): perform internal caching on getComputedStyle to boost …
fix($animate): perform internal caching on getComputedStyle to boost …