From d96c6914c7ed4efdabea8edc11d0c8ba64acb42b Mon Sep 17 00:00:00 2001 From: Ben Alpert Date: Tue, 14 Jan 2014 17:44:10 -0800 Subject: [PATCH] Make findComponentRoot faster with more nodeCache This is an alternative, less-invasive, fix for #891. Test Plan: On http://jsbin.com/OqOJidIQ/2/edit, got timings like [75, 56, 30, 36, 27, 27, 28, 32, 27, 27, 28, 31] instead of the old [75, 729, 46, 32, 28, 34, 26, 27, 27, 30, 26, 26]. I also added a counter to getID and saw it was called 3014 times instead of the old 636264. (3014 is the number of nodes (3000) plus 3 calls that happen for the initial render and 1 for each of the 11 renders after that.) --- src/core/ReactInstanceHandles.js | 24 ++++++++-- src/core/ReactMount.js | 75 +++++++++++++++++++++++--------- 2 files changed, 76 insertions(+), 23 deletions(-) diff --git a/src/core/ReactInstanceHandles.js b/src/core/ReactInstanceHandles.js index 4cc7c32bb4..1a78bc6d9e 100644 --- a/src/core/ReactInstanceHandles.js +++ b/src/core/ReactInstanceHandles.js @@ -168,7 +168,8 @@ function getFirstCommonAncestorID(oneID, twoID) { /** * Traverses the parent path between two IDs (either up or down). The IDs must - * not be the same, and there must exist a parent path between them. + * not be the same, and there must exist a parent path between them. If the + * callback returns `false`, traversal is stopped. * * @param {?string} start ID at which to start traversal. * @param {?string} stop ID at which to end traversal. @@ -197,10 +198,11 @@ function traverseParentPath(start, stop, cb, arg, skipFirst, skipLast) { var depth = 0; var traverse = traverseUp ? getParentID : getNextDescendantID; for (var id = start; /* until break */; id = traverse(id, stop)) { + var ret; if ((!skipFirst || id !== start) && (!skipLast || id !== stop)) { - cb(id, traverseUp, arg); + ret = cb(id, traverseUp, arg); } - if (id === stop) { + if (ret === false || id === stop) { // Only break //after// visiting `stop`. break; } @@ -296,6 +298,22 @@ var ReactInstanceHandles = { } }, + /** + * Traverse a node ID, calling the supplied `cb` for each ancestor ID. For + * example, passing `.r[0].{row-0}.[1]` would result in `cb` getting called + * with `.r[0]`, `.r[0].{row-0}`, and `.r[0].{row-0}.[1]`. + * + * NOTE: This traversal happens on IDs without touching the DOM. + * + * @param {string} targetID ID of the target node. + * @param {function} cb Callback to invoke. + * @param {*} arg Argument to invoke the callback with. + * @internal + */ + traverseAncestors: function(targetID, cb, arg) { + traverseParentPath('', targetID, cb, arg, true, false); + }, + /** * Exposed for unit testing. * @private diff --git a/src/core/ReactMount.js b/src/core/ReactMount.js index b44fa7bc0f..3780cd5193 100644 --- a/src/core/ReactMount.js +++ b/src/core/ReactMount.js @@ -163,6 +163,33 @@ function purgeID(id) { delete nodeCache[id]; } +var deepestNodeSoFar = null; +function findDeepestCachedAncestorImpl(ancestorID) { + var ancestor = nodeCache[ancestorID]; + if (ancestor && isValid(ancestor, ancestorID)) { + deepestNodeSoFar = ancestor; + } else { + // This node isn't populated in the cache, so presumably none of its + // descendants are. Break out of the loop. + return false; + } +} + +/** + * Return the deepest cached node whose ID is a prefix of `targetID`. + */ +function findDeepestCachedAncestor(targetID) { + deepestNodeSoFar = null; + ReactInstanceHandles.traverseAncestors( + targetID, + findDeepestCachedAncestorImpl + ); + + var foundNode = deepestNodeSoFar; + deepestNodeSoFar = null; + return foundNode; +} + /** * Mounting is the process of initializing a React component by creatings its * representative DOM elements and inserting them into a supplied `container`. @@ -502,46 +529,45 @@ var ReactMount = { }, /** - * Finds a node with the supplied `id` inside of the supplied `ancestorNode`. - * Exploits the ID naming scheme to perform the search quickly. + * Finds a node with the supplied `targetID` inside of the supplied + * `ancestorNode`. Exploits the ID naming scheme to perform the search + * quickly. * * @param {DOMEventTarget} ancestorNode Search from this root. - * @pararm {string} id ID of the DOM representation of the component. - * @return {DOMEventTarget} DOM node with the supplied `id`. + * @pararm {string} targetID ID of the DOM representation of the component. + * @return {DOMEventTarget} DOM node with the supplied `targetID`. * @internal */ - findComponentRoot: function(ancestorNode, id) { + findComponentRoot: function(ancestorNode, targetID) { var firstChildren = findComponentRootReusableArray; var childIndex = 0; - firstChildren[0] = ancestorNode.firstChild; + var deepestAncestor = findDeepestCachedAncestor(targetID) || ancestorNode; + + firstChildren[0] = deepestAncestor.firstChild; firstChildren.length = 1; while (childIndex < firstChildren.length) { var child = firstChildren[childIndex++]; + var targetChild; + while (child) { var childID = ReactMount.getID(child); if (childID) { - if (id === childID) { - // Emptying firstChildren/findComponentRootReusableArray is - // not necessary for correctness, but it helps the GC reclaim - // any nodes that were left at the end of the search. - firstChildren.length = 0; + // Even if we find the node we're looking for, we finish looping + // through its siblings to ensure they're cached so that we don't have + // to revisit this node again. Otherwise, we make n^2 calls to getID + // when visiting the many children of a single node in order. - return child; - } - - if (ReactInstanceHandles.isAncestorIDOf(childID, id)) { + if (targetID === childID) { + targetChild = child; + } else if (ReactInstanceHandles.isAncestorIDOf(childID, targetID)) { // If we find a child whose ID is an ancestor of the given ID, // then we can be sure that we only want to search the subtree // rooted at this child, so we can throw out the rest of the // search state. firstChildren.length = childIndex = 0; firstChildren.push(child.firstChild); - - // Ignore the rest of this child's siblings and immediately - // continue the outer loop with child.firstChild as child. - break; } } else { @@ -555,6 +581,15 @@ var ReactMount = { child = child.nextSibling; } + + if (targetChild) { + // Emptying firstChildren/findComponentRootReusableArray is + // not necessary for correctness, but it helps the GC reclaim + // any nodes that were left at the end of the search. + firstChildren.length = 0; + + return targetChild; + } } firstChildren.length = 0; @@ -564,7 +599,7 @@ var ReactMount = { 'findComponentRoot(..., %s): Unable to find element. This probably ' + 'means the DOM was unexpectedly mutated (e.g., by the browser). ' + 'Try inspecting the child nodes of the element with React ID `%s`.', - id, + targetID, ReactMount.getID(ancestorNode) ); },