Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Fiber] Minimize abuse of .alternate #6988

Merged
merged 2 commits into from
Jun 8, 2016
Merged

[Fiber] Minimize abuse of .alternate #6988

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c83a042
Minimize abuse of .alternate
sebmarkbage Jun 7, 2016
40180c4
Get rid of ugly and difficult to follow breaks in switch
sebmarkbage Jun 8, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
43 changes: 23 additions & 20 deletions src/renderers/shared/fiber/ReactChildFiber.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,22 +28,22 @@ 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;
}

switch (newChildren.$$typeof) {
case REACT_ELEMENT_TYPE: {
const element = (newChildren : ReactElement<any>);
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;
Expand Down Expand Up @@ -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 {
Expand All @@ -89,15 +91,14 @@ function createSubsequentChild(parent : Fiber, nextReusable : ?Fiber, previousSi
}
}

function createFirstChild(parent, newChildren) {
function createFirstChild(parent, existingChild, newChildren) {
if (typeof newChildren !== 'object' || newChildren === null) {
return null;
}

switch (newChildren.$$typeof) {
case REACT_ELEMENT_TYPE: {
const element = (newChildren : ReactElement<any>);
const existingChild : ?Fiber = parent.child;
if (existingChild &&
element.type === existingChild.type &&
element.key === existingChild.key) {
Expand Down Expand Up @@ -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;
Expand All @@ -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);
};
84 changes: 42 additions & 42 deletions src/renderers/shared/fiber/ReactFiberBeginWork.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,31 +27,30 @@ var {
YieldComponent,
} = ReactTypesOfWork;

function updateFunctionalComponent(workInProgress) {
var fn = workInProgress.type;
var props = workInProgress.input;
console.log('update fn:', fn.name);
var nextChildren = fn(props);

function reconcileChildren(current, workInProgress, nextChildren) {
workInProgress.child = ReactChildFiber.reconcileChildFibers(
workInProgress,
workInProgress.child,
current ? current.child : null,
nextChildren
);
}

function updateHostComponent(workInProgress) {
function updateFunctionalComponent(current, workInProgress) {
var fn = workInProgress.type;
var props = workInProgress.input;
console.log('update fn:', fn.name);
var nextChildren = fn(props);
reconcileChildren(current, workInProgress, nextChildren);
}

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);
Expand All @@ -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) {
Expand All @@ -101,40 +96,45 @@ function beginWork(workInProgress : Fiber) : ?Fiber {
}
switch (workInProgress.tag) {
case IndeterminateComponent:
mountIndeterminateComponent(workInProgress);
break;
mountIndeterminateComponent(current, workInProgress);
return workInProgress.child;
case FunctionalComponent:
updateFunctionalComponent(workInProgress);
break;
updateFunctionalComponent(current, workInProgress);
return workInProgress.child;
case ClassComponent:
console.log('class component', workInProgress.input.type.name);
break;
return workInProgress.child;
case HostComponent:
updateHostComponent(workInProgress);
break;
updateHostComponent(current, workInProgress);
return workInProgress.child;
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,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is null, we create a new fiber? How does it get attached?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That means that this child is new. We don't need an alternate until we try to update it. Remember that we only have one copy until we update. If later we try to update it, this get created through cloneFiber.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To put it another way: current is what already rendered on screen. workInProgress is what is about to be rendered. If a child is new, it has no current.

workInProgress.child
);
}
break;
return workInProgress.child;
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:
throw new Error('Unknown unit of work tag');
}
return workInProgress.child;
}

exports.beginWork = beginWork;
9 changes: 5 additions & 4 deletions src/renderers/shared/fiber/ReactFiberCompleteWork.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand All @@ -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);
Expand All @@ -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.
Expand Down
18 changes: 14 additions & 4 deletions src/renderers/shared/fiber/ReactFiberReconciler.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,12 @@ module.exports = function<T, P, I>(config : HostConfig<T, P, I>) : 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;
Expand All @@ -74,7 +79,12 @@ module.exports = function<T, P, I>(config : HostConfig<T, P, I>) : 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;
Expand Down Expand Up @@ -123,8 +133,8 @@ module.exports = function<T, P, I>(config : HostConfig<T, P, I>) : 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 {};
}

Expand Down