From c83a0428f126e9f41d199b56fe8bf457dce5dad2 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Tue, 7 Jun 2016 15:02:56 -0700 Subject: [PATCH 1/2] Minimize abuse of .alternate This avoids using .alternate as much as possible and isolates it to the root. For anything that is "work in progress" this happens to be the same as the alternate field. To avoid an extra "current" field on work in progress fibers, we can just use the alternate. The timing for when this is true might be a bit tricky to reason about so I explicitly pass the current value everywhere from the top. That way we can always change this in the future to use an explicit field or we can try to maintain a parallel data structure that remembers which was the "current" fiber for each wip child. --- src/renderers/shared/fiber/ReactChildFiber.js | 43 ++++++----- .../shared/fiber/ReactFiberBeginWork.js | 73 ++++++++++--------- .../shared/fiber/ReactFiberCompleteWork.js | 9 ++- .../shared/fiber/ReactFiberReconciler.js | 18 ++++- 4 files changed, 79 insertions(+), 64 deletions(-) diff --git a/src/renderers/shared/fiber/ReactChildFiber.js b/src/renderers/shared/fiber/ReactChildFiber.js index b629dbd0a1..8921b9f188 100644 --- a/src/renderers/shared/fiber/ReactChildFiber.js +++ b/src/renderers/shared/fiber/ReactChildFiber.js @@ -28,7 +28,7 @@ var { var ReactFiber = require('ReactFiber'); var ReactReifiedYield = require('ReactReifiedYield'); -function createSubsequentChild(parent : Fiber, nextReusable : ?Fiber, previousSibling : Fiber, newChildren) : Fiber { +function createSubsequentChild(parent : Fiber, existingChild : ?Fiber, previousSibling : Fiber, newChildren) : Fiber { if (typeof newChildren !== 'object' || newChildren === null) { return previousSibling; } @@ -36,14 +36,14 @@ function createSubsequentChild(parent : Fiber, nextReusable : ?Fiber, previousSi switch (newChildren.$$typeof) { case REACT_ELEMENT_TYPE: { const element = (newChildren : ReactElement); - if (nextReusable && - element.type === nextReusable.type && - element.key === nextReusable.key) { + if (existingChild && + element.type === existingChild.type && + element.key === existingChild.key) { // TODO: This is not sufficient since previous siblings could be new. // Will fix reconciliation properly later. - const clone = ReactFiber.cloneFiber(nextReusable); + const clone = ReactFiber.cloneFiber(existingChild); clone.input = element.props; - clone.child = nextReusable.child; + clone.child = existingChild.child; clone.sibling = null; previousSibling.sibling = clone; return clone; @@ -75,12 +75,14 @@ function createSubsequentChild(parent : Fiber, nextReusable : ?Fiber, previousSi if (Array.isArray(newChildren)) { let prev : Fiber = previousSibling; + let existing : ?Fiber = existingChild; for (var i = 0; i < newChildren.length; i++) { - let reusable = null; - if (prev.alternate) { - reusable = prev.alternate.sibling; + prev = createSubsequentChild(parent, existing, prev, newChildren[i]); + if (prev && existing) { + // TODO: This is not correct because there could've been more + // than one sibling consumed but I don't want to return a tuple. + existing = existing.sibling; } - prev = createSubsequentChild(parent, reusable, prev, newChildren[i]); } return prev; } else { @@ -89,7 +91,7 @@ function createSubsequentChild(parent : Fiber, nextReusable : ?Fiber, previousSi } } -function createFirstChild(parent, newChildren) { +function createFirstChild(parent, existingChild, newChildren) { if (typeof newChildren !== 'object' || newChildren === null) { return null; } @@ -97,7 +99,6 @@ function createFirstChild(parent, newChildren) { switch (newChildren.$$typeof) { case REACT_ELEMENT_TYPE: { const element = (newChildren : ReactElement); - const existingChild : ?Fiber = parent.child; if (existingChild && element.type === existingChild.type && element.key === existingChild.key) { @@ -136,16 +137,18 @@ function createFirstChild(parent, newChildren) { if (Array.isArray(newChildren)) { var first : ?Fiber = null; var prev : ?Fiber = null; + var existing : ?Fiber = existingChild; for (var i = 0; i < newChildren.length; i++) { if (prev == null) { - prev = createFirstChild(parent, newChildren[i]); + prev = createFirstChild(parent, existing, newChildren[i]); first = prev; } else { - let reusable = null; - if (prev.alternate) { - reusable = prev.alternate.sibling; - } - prev = createSubsequentChild(parent, reusable, prev, newChildren[i]); + prev = createSubsequentChild(parent, existing, prev, newChildren[i]); + } + if (prev && existing) { + // TODO: This is not correct because there could've been more + // than one sibling consumed but I don't want to return a tuple. + existing = existing.sibling; } } return first; @@ -155,6 +158,6 @@ function createFirstChild(parent, newChildren) { } } -exports.reconcileChildFibers = function(parent : Fiber, firstChild : ?Fiber, newChildren : ReactNodeList) : ?Fiber { - return createFirstChild(parent, newChildren); +exports.reconcileChildFibers = function(parent : Fiber, currentFirstChild : ?Fiber, newChildren : ReactNodeList) : ?Fiber { + return createFirstChild(parent, currentFirstChild, newChildren); }; diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index 1cf9081599..6b534789ab 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -27,31 +27,30 @@ var { YieldComponent, } = ReactTypesOfWork; -function updateFunctionalComponent(workInProgress) { +function reconcileChildren(current, workInProgress, nextChildren) { + workInProgress.child = ReactChildFiber.reconcileChildFibers( + workInProgress, + current ? current.child : null, + nextChildren + ); +} + +function updateFunctionalComponent(current, workInProgress) { var fn = workInProgress.type; var props = workInProgress.input; console.log('update fn:', fn.name); var nextChildren = fn(props); - - workInProgress.child = ReactChildFiber.reconcileChildFibers( - workInProgress, - workInProgress.child, - nextChildren - ); + reconcileChildren(current, workInProgress, nextChildren); } -function updateHostComponent(workInProgress) { +function updateHostComponent(current, workInProgress) { console.log('host component', workInProgress.type, typeof workInProgress.input.children === 'string' ? workInProgress.input.children : ''); var nextChildren = workInProgress.input.children; - workInProgress.child = ReactChildFiber.reconcileChildFibers( - workInProgress, - workInProgress.child, - nextChildren - ); + reconcileChildren(current, workInProgress, nextChildren); } -function mountIndeterminateComponent(workInProgress) { +function mountIndeterminateComponent(current, workInProgress) { var fn = workInProgress.type; var props = workInProgress.input; var value = fn(props); @@ -64,34 +63,30 @@ function mountIndeterminateComponent(workInProgress) { // Proceed under the assumption that this is a functional component workInProgress.tag = FunctionalComponent; } - workInProgress.child = ReactChildFiber.reconcileChildFibers( - workInProgress, - workInProgress.child, - value - ); + reconcileChildren(current, workInProgress, value); } -function updateCoroutineComponent(workInProgress) { +function updateCoroutineComponent(current, workInProgress) { var coroutine = (workInProgress.input : ?ReactCoroutine); if (!coroutine) { throw new Error('Should be resolved by now'); } console.log('begin coroutine', workInProgress.type.name); - workInProgress.child = ReactChildFiber.reconcileChildFibers( - workInProgress, - workInProgress.child, - coroutine.children - ); + reconcileChildren(current, workInProgress, coroutine.children); } -function beginWork(workInProgress : Fiber) : ?Fiber { - const alt = workInProgress.alternate; - if (alt && workInProgress.input === alt.memoizedInput) { +function beginWork(current : ?Fiber, workInProgress : Fiber) : ?Fiber { + // The current, flushed, state of this fiber is the alternate. + // Ideally nothing should rely on this, but relying on it here + // means that we don't need an additional field on the work in + // progress. + if (current && workInProgress.input === current.memoizedInput) { // The most likely scenario is that the previous copy of the tree contains // the same input as the new one. In that case, we can just copy the output // and children from that node. - workInProgress.output = alt.output; - workInProgress.child = alt.child; + workInProgress.output = current.output; + workInProgress.child = current.child; + workInProgress.stateNode = current.stateNode; return null; } if (workInProgress.input === workInProgress.memoizedInput) { @@ -101,34 +96,40 @@ function beginWork(workInProgress : Fiber) : ?Fiber { } switch (workInProgress.tag) { case IndeterminateComponent: - mountIndeterminateComponent(workInProgress); + mountIndeterminateComponent(current, workInProgress); break; case FunctionalComponent: - updateFunctionalComponent(workInProgress); + updateFunctionalComponent(current, workInProgress); break; case ClassComponent: console.log('class component', workInProgress.input.type.name); break; case HostComponent: - updateHostComponent(workInProgress); + updateHostComponent(current, workInProgress); break; case CoroutineHandlerPhase: // This is a restart. Reset the tag to the initial phase. workInProgress.tag = CoroutineComponent; // Intentionally fall through since this is now the same. case CoroutineComponent: - updateCoroutineComponent(workInProgress); + updateCoroutineComponent(current, workInProgress); // This doesn't take arbitrary time so we could synchronously just begin // eagerly do the work of workInProgress.child as an optimization. if (workInProgress.child) { - return beginWork(workInProgress.child); + return beginWork( + workInProgress.child.alternate, + workInProgress.child + ); } break; case YieldComponent: // A yield component is just a placeholder, we can just run through the // next one immediately. if (workInProgress.sibling) { - return beginWork(workInProgress.sibling); + return beginWork( + workInProgress.sibling.alternate, + workInProgress.sibling + ); } return null; default: diff --git a/src/renderers/shared/fiber/ReactFiberCompleteWork.js b/src/renderers/shared/fiber/ReactFiberCompleteWork.js index 1ec3e0c1e3..f4d72e3d10 100644 --- a/src/renderers/shared/fiber/ReactFiberCompleteWork.js +++ b/src/renderers/shared/fiber/ReactFiberCompleteWork.js @@ -54,7 +54,7 @@ function recursivelyFillYields(yields, output : ?Fiber | ?ReifiedYield) { } } -function moveCoroutineToHandlerPhase(workInProgress : Fiber) { +function moveCoroutineToHandlerPhase(current : ?Fiber, workInProgress : Fiber) { var coroutine = (workInProgress.input : ?ReactCoroutine); if (!coroutine) { throw new Error('Should be resolved by now'); @@ -81,15 +81,16 @@ function moveCoroutineToHandlerPhase(workInProgress : Fiber) { var props = coroutine.props; var nextChildren = fn(props, yields); + var currentFirstChild = current ? current.stateNode : null; workInProgress.stateNode = ReactChildFiber.reconcileChildFibers( workInProgress, - workInProgress.stateNode, + currentFirstChild, nextChildren ); return workInProgress.stateNode; } -exports.completeWork = function(workInProgress : Fiber) : ?Fiber { +exports.completeWork = function(current : ?Fiber, workInProgress : Fiber) : ?Fiber { switch (workInProgress.tag) { case FunctionalComponent: console.log('/functional component', workInProgress.type.name); @@ -104,7 +105,7 @@ exports.completeWork = function(workInProgress : Fiber) : ?Fiber { break; case CoroutineComponent: console.log('/coroutine component', workInProgress.input.handler.name); - return moveCoroutineToHandlerPhase(workInProgress); + return moveCoroutineToHandlerPhase(current, workInProgress); case CoroutineHandlerPhase: transferOutput(workInProgress.stateNode, workInProgress); // Reset the tag to now be a first phase coroutine. diff --git a/src/renderers/shared/fiber/ReactFiberReconciler.js b/src/renderers/shared/fiber/ReactFiberReconciler.js index 18a234d727..e7f0e2a441 100644 --- a/src/renderers/shared/fiber/ReactFiberReconciler.js +++ b/src/renderers/shared/fiber/ReactFiberReconciler.js @@ -51,7 +51,12 @@ module.exports = function(config : HostConfig) : Reconciler { function completeUnitOfWork(workInProgress : Fiber) : ?Fiber { while (true) { - var next = completeWork(workInProgress); + // The current, flushed, state of this fiber is the alternate. + // Ideally nothing should rely on this, but relying on it here + // means that we don't need an additional field on the work in + // progress. + const current = workInProgress.alternate; + const next = completeWork(current, workInProgress); if (next) { // If completing this work spawned new work, do that next. return next; @@ -74,7 +79,12 @@ module.exports = function(config : HostConfig) : Reconciler { } function performUnitOfWork(workInProgress : Fiber) : ?Fiber { - var next = beginWork(workInProgress); + // The current, flushed, state of this fiber is the alternate. + // Ideally nothing should rely on this, but relying on it here + // means that we don't need an additional field on the work in + // progress. + const current = workInProgress.alternate; + const next = beginWork(current, workInProgress); if (next) { // If this spawns new work, do that next. return next; @@ -123,8 +133,8 @@ module.exports = function(config : HostConfig) : Reconciler { // TODO: Unify this with ReactChildFiber. We can't now because the parent // is passed. Should be doable though. Might require a wrapper don't know. if (rootFiber && rootFiber.type === element.type && rootFiber.key === element.key) { - nextUnitOfWork = rootFiber; - rootFiber.input = element.props; + nextUnitOfWork = ReactFiber.cloneFiber(rootFiber); + nextUnitOfWork.input = element.props; return {}; } From 40180c4b268654d793289cc567b9e5bcb91766ad Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Tue, 7 Jun 2016 17:05:25 -0700 Subject: [PATCH 2/2] Get rid of ugly and difficult to follow breaks in switch Because pattern matching or something. --- src/renderers/shared/fiber/ReactFiberBeginWork.js | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index 6b534789ab..24ec08db90 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -97,16 +97,16 @@ function beginWork(current : ?Fiber, workInProgress : Fiber) : ?Fiber { switch (workInProgress.tag) { case IndeterminateComponent: mountIndeterminateComponent(current, workInProgress); - break; + return workInProgress.child; case FunctionalComponent: updateFunctionalComponent(current, workInProgress); - break; + return workInProgress.child; case ClassComponent: console.log('class component', workInProgress.input.type.name); - break; + return workInProgress.child; case HostComponent: updateHostComponent(current, workInProgress); - break; + return workInProgress.child; case CoroutineHandlerPhase: // This is a restart. Reset the tag to the initial phase. workInProgress.tag = CoroutineComponent; @@ -121,7 +121,7 @@ function beginWork(current : ?Fiber, workInProgress : Fiber) : ?Fiber { workInProgress.child ); } - break; + return workInProgress.child; case YieldComponent: // A yield component is just a placeholder, we can just run through the // next one immediately. @@ -135,7 +135,6 @@ function beginWork(current : ?Fiber, workInProgress : Fiber) : ?Fiber { default: throw new Error('Unknown unit of work tag'); } - return workInProgress.child; } exports.beginWork = beginWork;