[4.1.0] NodeInterface.removeAll - performance increase

**westy** · 29 Aug 2012 1:00 AM

Hi,

Been looking into why, in IE7, refreshing a tree node can take over 30 seconds with around 200 child nodes.
This causes the infamous, 'this script is taking too long, do you want to stop it' message (or words to that effect) four or fives times!

It appears that the main culprit is NodeInterface.removeAll; it's simplistic approach is to call remove in a loop.
The remove method does a lot of processing that is not needed in the removeAll case, since we know we are removing ALL children.

Anyway, the following reduces the time spent from around 30 seconds (on my i7 desktop with 12Gb and a fast SSD) to less than a second.
I'm not sure it's perfect, but it seems to work okay for me.

Code:

/**
 * Removes all child nodes from this node.
 * This is my override to speed up the processing but a huge amount... previous version simply calls remove,
 * and does a lot of pointless housekeeping, since in this case we know we are removing ALL nodes.
 * @param {Boolean} [destroy=false] True to destroy the node upon removal.
 * @return {Ext.data.NodeInterface} this
 */
removeAll : function(destroy, suppressEvents) {
    var me = this,
        childNodes = me.childNodes,
        node,
        index,
        haveRemoveListeners = me.hasListeners.remove;


    while ((node = childNodes[childNodes.length - 1])) {
        index = childNodes.length - 1;


        // Remove it from childNodes collection
        delete childNodes[index];
        childNodes.length = index;


        // Inform any listeners
        if ((suppressEvents !== true) && haveRemoveListeners) {
            me.fireEvent("remove", me, node, false);
        }


        if (destroy) {
            node.destroy(true);
        } else {
            node.clear();
        }
    }


    // This node will no longer have any child nodes, so update ourselves.
    me.set('loaded', false);


    return me;
}

Now, I'm not 100% sure it's doing everything needed, but seems to work okay for me (and I have a lot of complex trees). Ideally I'd like one event at the end, rather than one for each node, but think it's needed with the current set up...

I considered setting 'expanded' to false at the end as well, since when removeAll is called in isolation this puts the tree back into a nice state, but this seems to break the load process, where removeAll is called if clearOnLoad is set.

Since NodeInterface is a complete pain to override I'll include that too since others may want to do some testing with it too!
Also includes my fix for destroy, and you can see the original remove method, that I used as a basis for the innards of removeAll.

Code:

Ext.require('Ext.data.NodeInterface',
    function() {
        Ext.override(Ext.data.NodeInterface, {
            statics: {
                /**
                 * Fix to reset childNodes to [] rather than null.
                 * See: http://www.sencha.com/forum/showthread.php?203853-TreeStore-bugs-appendChild()-load()
                 */
                decorate: function(modelClass) {
                    this.callParent(arguments);


                    // Replacing implementation of destroy, supplied from getPrototypeBody.
                    modelClass.override({
                        /**
                         * Destroys the node.
                         */
                        destroy : function(silent) {
                            /*
                             * Silent is to be used in a number of cases
                             * 1) When setRoot is called.
                             * 2) When destroy on the tree is called
                             * 3) For destroying child nodes on a node
                             */
                            var me      = this,
                                options = me.destroyOptions,
                                nodes   = me.childNodes,
                                nLen    = nodes.length,
                                n;


                            if (silent === true) {
                                me.clear(true);


                                for (n = 0; n < nLen; n++) {
                                    nodes[n].destroy(true);
                                }


                                me.childNodes = [];         // <WestyFix> This was getting set to null.
                                delete me.destroyOptions;
                                me.callOverridden([options]);
                            } else {
                                me.destroyOptions = silent;
                                // overridden method will be called, since remove will end up calling destroy(true);
                                me.remove(true);
                            }
                        },


                        /**
                         * Removes all child nodes from this node.
                         * This is my override to speed up the processing but a huge amount... previous version simply calls remove,
                         * and does a lot of pointless housekeeping, since in this case we know we are removing ALL nodes.
                         * @param {Boolean} [destroy=false] True to destroy the node upon removal.
                         * @return {Ext.data.NodeInterface} this
                         */
                        removeAll : function(destroy, suppressEvents) {
                            var me = this,
                                childNodes = me.childNodes,
                                node,
                                index,
                                haveRemoveListeners = me.hasListeners.remove;


                            while ((node = childNodes[childNodes.length - 1])) {
                                index = childNodes.length - 1;


                                // Remove it from childNodes collection
                                delete childNodes[index];
                                childNodes.length = index;


                                // Inform any listeners
                                if ((suppressEvents !== true) && haveRemoveListeners) {
                                    me.fireEvent("remove", me, node, false);
                                }


                                if (destroy) {
                                    node.destroy(true);
                                } else {
                                    node.clear();
                                }
                            }


                            // This node will no longer have any child nodes, so update ourselves.
                            me.set('loaded', false);


                            return me;
                        }


                        // removeChild : function(node, destroy, suppressEvents, isMove) {
                        //     var me = this,
                        //         index = me.indexOf(node),
                        //         i, childCount;


                        //     if (index == -1 || (suppressEvents !== true && (!me.hasListeners.beforeremove || me.fireEvent("beforeremove", me, node, !!isMove) === false))) {
                        //         return false;
                        //     }


                        //     // remove it from childNodes collection
                        //     Ext.Array.erase(me.childNodes, index, 1);


                        //     // update child refs
                        //     if (me.firstChild == node) {
                        //         me.setFirstChild(node.nextSibling);
                        //     }
                        //     if (me.lastChild == node) {
                        //         me.setLastChild(node.previousSibling);
                        //     }


                        //     // update siblings
                        //     if (node.previousSibling) {
                        //         node.previousSibling.nextSibling = node.nextSibling;
                        //     }
                        //     if (node.nextSibling) {
                        //         node.nextSibling.previousSibling = node.previousSibling;
                        //     }


                        //     // update the info for all siblings starting at the index before the node's old index (or 0 if the removed node was the firstChild)
                        //     for(i = index > 0 ? index - 1 : 0, childCount = me.childNodes.length; i < childCount; i++) {
                        //         me.childNodes[i].updateInfo();
                        //     }


                        //     // If this node suddenly doesnt have childnodes anymore, update myself
                        //     if (!me.childNodes.length) {
                        //         me.set('loaded', me.isLoaded());
                        //     }


                        //     if (suppressEvents !== true) {
                        //         if (me.hasListeners.remove) {
                        //             me.fireEvent("remove", me, node, !!isMove);
                        //         }
                        //     }


                        //     if (destroy) {
                        //         node.destroy(true);
                        //     } else {
                        //         node.clear();
                        //     }


                        //     return node;
                        // }
                    });
                }
            }
        });
    }
);

Hope the author of NodeInterface can be suitably punished for this horrible horrible 'class'

Oh, and apologies for not using the bug reporting form (again), but I don't think it really applies in this case...

Regards,
Westy

PS: Please, post any thoughts or improvements here.
I'm now off to try and speed up node creation, will report back if find any magic bullets