Improve tree rendering performance

[JakeSummers]

I have a tree in my app that has 500 items. Unfortunately, I am running into CPU performance issues because each of the items in the tree corresponds to a new Angular2 component. Running change detection on each of these components is slow and my app is becoming unresponsive.

I am planning on working around this problem by modifying my TreeNodes get children getter/method to only return items when the tree node is expanded, however this feels like a hack. It seems to me like the tree should only create components (and put items in the dom) for TreeNodes that are currently expanded.

I think that this may be fairly straight-forward to implement. Currently the p-treeNode looks like this:

    <li class="ui-treenode" *ngIf="node">
        <div class="ui-treenode-content" [ngClass]="{'ui-treenode-selectable': tree.selectionMode}" 
            ...
        </div>
        <ul class="ui-treenode-children" style="display: none;" *ngIf="node.children" [style.display]="node.expanded ? 'block' : 'none'">
            <p-treeNode *ngFor="let childNode of node.children" [node]="childNode"></p-treeNode>
        </ul>
    </li>

I think that updating it to this, would be much faster:

    <li class="ui-treenode" *ngIf="node">
        <div class="ui-treenode-content" [ngClass]="{'ui-treenode-selectable': tree.selectionMode}" 
            ...
        </div>
        <!-- CHANGE ON THIS LINE: -->
        <ul class="ui-treenode-children" style="display: none;" *ngIf="node.children && node.expanded">
            <p-treeNode *ngFor="let childNode of node.children" [node]="childNode"></p-treeNode>
        </ul>
    </li>

I haven't tested this change but I would happy test it and submit a pull-request tomorrow, if we could get this change in by Wednesday next week (Oct 26). My project is going final very shortly and having this functionality (in a non-hacky way) would be very beneficial.

[JakeSummers]

Flagging @cagataycivici - if I submit a pull request on Friday Oct 21, can you merge it in before next Wednesday?

[JakeSummers]

Profiling info

I created two CPU profiles using the chrome profiler. During each, I ran the profiler for 45 seconds while moving the mouse over the app to trigger change detection.

In the first test, the tree contained 500 items and completing one change detection iteration took up to 100ms:

When the tree contains only 15 items completing one change detection iteration was much much faster. It typically took around 8ms, however you can see that there are long stretches of idle time between the change detection cycles:

[cagataycivici]

Please do a PR and we'll review. At first glance this makes sense for sure.

[JakeSummers]

Thanks for the response. Moved onto a new project so won't have time to do a PR for this.

For anyone else with similar issues, I hacked my TreeNode to do the following:

export class JakeTreeNode implements TreeNode {

    ...

    get children(): FolderTreeNode[] {
        if(!this.expanded){
            return [];
        }
        return this._children;
    }

}

There are some other usecases, that you have to handle. For example, when the selected node is collapsed, you may want to reset the selection since the Tree will no longer have the selected node. This seems like an invalid state.

[cagataycivici]

Thank you, I'll review.