HabitRPG / habitica

A habit tracker app which treats your goals like a Role Playing Game.
https://habitica.com
Other
11.81k stars 4.05k forks source link

Chat performance is not as good as it should be #1884

Closed snicker closed 9 years ago

snicker commented 10 years ago

Chat is horrendously slow on most mobile platforms, and I think it has to do with one major factor: the ngClass directive is validated everytime a model is set to "dirty" and Angular runs $digest. This causes every single chat message to be tested twice on the input of every single character into a text box.

Check out this fiddle for what I'm talking about: http://jsfiddle.net/HB7LU/1070/

I'm going to propose a couple potential solutions to get some idea about what others think:

Using this issue as a discussion/suggestions area for the most part

paglias commented 10 years ago

I still have to understand why messages are check when the text in the chat box is updated, they shouldn't be dependent on it and so angular should check only for $scope properties linked to the new message being written. Anyway I had heard about bindonce a while ago and I think it would be really really great

AdrienLemaire commented 10 years ago

Maybe related to #1850

snicker commented 10 years ago

from the Angular docs:

"At the end $apply, Angular performs a $digest cycle on the root scope, which then propagates throughout all child scopes. During the $digest cycle, all $watched expressions or functions are checked for model mutation and if a mutation is detected, the $watch listener is called."

for any kind of binding, a $watch is created. So the above is why it happens. It's behavior that is pivotal for Angular to be able to do what it does, but it does create some unnecessary overhead for static things like this. I think I'm going to conquer bindonce implementation and see how it fares

snicker commented 10 years ago

bindonce implemented with this PR: https://github.com/HabitRPG/habitrpg/pull/1885

snicker commented 10 years ago

So I think we might want to put some "best practices" somewhere for use of Bindonce.

Bindonce directives should only be used if the following rule is true: the model data being used for the conditional will not change during a session, or will change infrequently enough that it's not unreasonable to expect the user to refresh the page.

If you do that, we can see some HUGE performance benefits.

AdrienLemaire commented 10 years ago

@snicker please add a tip for this in the wiki, thanks

You can add a new section best practices :)

snicker commented 10 years ago

@Fandekasp added: http://habitrpg.wikia.com/wiki/Guidance_for_Contributors#Angular.2FNode.2FJade_Tips_.26_Best_Practices

AdrienLemaire commented 10 years ago

yep, I saw that, nice tip thanks ! I'll try to update my audio code to use bindonce later then :)

snicker commented 10 years ago

Blech, bindonce seemed a lot more promising than it is turning out to be. Several intense issues, we can use it a bit for now, but there are strange behaviors: https://github.com/Pasvaz/bindonce/issues/35 https://github.com/Pasvaz/bindonce/issues/29

paglias commented 10 years ago

@snicker do you know wheter bo-text directive is working without issues?

snicker commented 10 years ago

Unsure. I would recommend testing to verify.

negue commented 9 years ago

@snicker Last month I added some "track by" performance tricks to some lists, one of them was the chat, for me it is faster again.

Could you test it too?