iVantage / angular-ivh-treeview

A treeview for AngularJS with filtering and checkbox support.
http://ivantage.github.io/angular-ivh-treeview/
MIT License
239 stars 89 forks source link

Watch values rather than functions #69

Open jtrussell opened 9 years ago

jtrussell commented 9 years ago

Watching functions is slow, and we do it often.

icfantv commented 8 years ago

What would you think about making the directive responsible for adding/removing a class based on an event (click) rather than using watches to be notified of when a node changes which then set a property on the node (when then have an ng-class to assign the value? This would remove two watches per node?

We have a large tree of television networks (973 leaf nodes) such that affiliates are grouped by parent - e.g., the ABC parent node will have 28 children nodes. With this setup, the default tree creates 13,563 watches. Overriding the node template and using one-time binding syntax drops it to 10,452. Adding an ng-if to the ivh-treeview-children <div> actually creates more watches (63 in our case) but I can't speak to the performance tradeoff mentioned in your documentation.

Tree traversal is at most an O(n) operation assuming you only visit each node once. I need to learn how your directives are structured better to see if we can leverage the observer pattern on parent directives to provide this functionality. But this should reduce the number of watches SIGNIFICANTLY - with the expense of visiting each tree node once.

Thoughts?

jtrussell commented 8 years ago

Could definitely be interesting. IIRC there are maybe 15 watchers/node with the default template. Cutting out a handful of watchers per node won't buy you tons of headroom but it certainly won't hurt. At some level just using ng-repeat internally and the fact that each node has an isolate scope is going to be a bottleneck. In a prior life this module used an evented system for managing updates but we haven't tried implementing a more explicit observer pattern, I'd be all ears if you have suggestions though I'd really love to get #84 finished before too much optimization happens.

I'm really surprised that adding the ng-if increases your watch count. If I'm remembering the watchers/node ratio right that would mean you typically have at least ~93% of your nodes visible at a given time, does that sound about right?

icfantv commented 8 years ago

I looked at #62 and had applied the wrong controller method to the ivh-treeview-children node via ng-if - using the right method helps significantly, 4400 watches on initial load. We have the networks split into Broadcast and On Demand w/ the Broadcast tree open by default. It does have the bulk of the networks so that would explain high number of initial watches. Your number sounds about right, if a little high - I am using a custom node template and have done some more optimizations (it's a CB tree, so I don't need that ng-if, for example).

I would agree that using events is not the right way to go and you're right in that just building the DOM up for a large tree is the bottleneck. I'm wondering if using the shadow DOM is the right way to go here to speed things up and separate the styling from the DOM structure...or if it would make a difference.