ngMobile / directive / ngClick.js - Dramatic performance

[FelikZ]

I had to stop using mobile ngClick, because of its problems with performance.

In my case, the 300 elements that have nested 3-4 ngClick, while adding new elements, performance (especially in chrome) tend to zero - its not possible to work with the application then. What does not happen if you use a standard ngClick.

[revolunet]

could be interesting to have a plunker to play with

[bshepherdson]

Interesting. I'm not surprised there were problems here. When you touch to scroll, it will fire touchstart and then dozens or hundreds of touchmoves as you drag, hitting various elements. If they're nested 3-4 deep and densely packed next to each other, I'm not surprised that trying to call hundreds (thousands?) of touchmove callbacks per second is slow, even if they are short.

This doesn't happen with regular ngClick, since it isn't activated until you do click. Something you can try: use normal ngClick (ie. don't load ngMobile) and try fastclick.js. Does it have the same problem? If not, I'll take a look at its code and see if there's a way to make ngMobile faster in this case.

[FelikZ]

@shepheb I will try to check fastclick.js on weekends and post feedback.

[FelikZ]

@shepheb so the problem is in element.bind('<touch_event_here>') calls in ngMobile ngClick directive definition. I'd commented them and performance become smooth. On each call the performance of binding touch events corrupted incrementally. I trying to search implementation of that method, but no luck. Is that default jQuery's behavior?

[bshepherdson]

That doesn't surprise me. The number of calls to element.bind('foo') grows linearly with the number of ng-click="..." directives on the page. I'm not certain, but they should be removed properly when those elements go out of scope. The browser should remove event handlers from any elements that are destroyed when you switch views.

So if you have hundreds of ng-clickable elements on the page, there are going to be hundreds of event handlers created. The difference in performance between normal ngClick and ngMobile's ngClick is that the mobile version needs to handle touchmoves, of which there are very many while scrolling (on iOS at least), but desktop only cares about click events.

Did you try fastclick.js without ngMobile included? I want to know if it suffers from the same problem.

[FelikZ]

@shepheb no I doesn't try fastclick.js yet, BUT the issue is definitely in jQuery's bind! I try'ed, for test purposes, to replace touch* events to mouse* events and then there is no performance issues. I'm using jQuery 1.9.2 and then updated it to 1.10.2 but problem is still reproduced. I will try to debug deeply after my vacation.

[revolunet]

don't you think the ngClick directive should listen to scope.$destroy and then remove the listener ?

angular.js/src/ng/directive/ngEventDirs.js

Line 43 in 2bb27d4

ngEventDirectives[directiveName] = ['$parse', function($parse) {

[FelikZ]

@revolunet, I think it should.

BTW, I spend a few hours to debug deeper jQuery's bind and profiling my project and I'd found the endpoint issue! The problem is in native addEventListener function. There is a topic on stackoverflow. So, its reproduced only at desktop Chrome (Latest 29.0.1547.57 version).

The only thing I came up with is to call addEventListener only on visible to user items and call removeEventListener to not visible items when user's viewport changes.

Also I can't understand why ngMobile.ngClick binds those events even if device does not support touch events. I think it shouldn't bind them.

[bshepherdson]

BIzarre. Great work finding the root bug!

We can probably add a touch support check. I had been assuming that including ngMobile, or ngTouch as it's known these days, happens only on touchy devices anyway. But perhaps that was naive; I can add that snippet to check for touch event support before doing anything in ngTouch's ngClick.

[FelikZ]

@shepheb it would be awesome. As a quick solution I would rather use Modernizr.touch check before module initialization and do not require ngMobile if no touch was detected.

[dehru]

+1

[sudhakar]

+1

[CWSpear]

+1

[akofman]

+1