Skip to content

Commit

Permalink
Move memoization to begin phase
Browse files Browse the repository at this point in the history 
Currently we update the memoized inputs (props, state) during the
complete phase, as we go back up the tree. That means we can't reuse
work until of its children have completed.

By moving memoization to the begin phase, we can do a shallow bailout,
reusing a unit of work even if there's still work to do in its children.

Memoization now happens whenever a fiber's `child` property is updated;
typically, right after reconciling. It's also updated when
`shouldComponentUpdate` returns false, because that indicates that the
given state and props are equal to the memoized state and props.

Includes a test that confirms that work that is bailed out before
completing can be reused without dropping the entire subtree.
  • Loading branch information
@acdlite
acdlite committed Jan 8, 2017
1 parent f589274 commit ef96a54
Show file tree
Hide file tree
Showing 8 changed files with 255 additions and 86 deletions.
3 changes: 0 additions & 3 deletions scripts/fiber/tests-failing.txt
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
src/addons/__tests__/ReactComponentWithPureRenderMixin-test.js
* does not do a deep comparison

src/addons/__tests__/ReactFragment-test.js
* should throw if a plain object is used as a child
* should throw if a plain object even if it is in an owner
Expand Down
3 changes: 3 additions & 0 deletions scripts/fiber/tests-passing.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ scripts/error-codes/__tests__/invertObject-test.js

src/addons/__tests__/ReactComponentWithPureRenderMixin-test.js
* provides a default shouldComponentUpdate implementation
* does not do a deep comparison

src/addons/__tests__/ReactFragment-test.js
* warns for numeric keys on objects as children
Expand Down Expand Up @@ -1141,7 +1142,9 @@ src/renderers/shared/fiber/__tests__/ReactIncremental-test.js
* can resume work in a subtree even when a parent bails out
* can resume work in a bailed subtree within one pass
* can reuse work done after being preempted
* can reuse work that began but did not complete, after being preempted
* can reuse work if shouldComponentUpdate is false, after being preempted
* memoizes work even if shouldComponentUpdate returns false
* can update in the middle of a tree using setState
* can queue multiple state updates
* can use updater form of setState
Expand Down
101 changes: 69 additions & 32 deletions src/renderers/shared/fiber/ReactFiberBeginWork.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ var {
OffscreenPriority,
} = require('ReactPriorityLevel');
var {
Update,
Placement,
ContentReset,
Err,
Expand Down Expand Up @@ -87,7 +86,12 @@ module.exports = function<T, P, I, TI, C, CX, CI>(
mountClassInstance,
resumeMountClassInstance,
updateClassInstance,
} = ReactFiberClassComponent(scheduleUpdate, getPriorityContext);
} = ReactFiberClassComponent(
scheduleUpdate,
getPriorityContext,
memoizeProps,
memoizeState,
);

function markChildAsProgressed(current, workInProgress, priorityLevel) {
// We now have clones. Let's store them as the currently progressed work.
Expand Down Expand Up @@ -172,12 +176,13 @@ module.exports = function<T, P, I, TI, C, CX, CI>(
// Normally we can bail out on props equality but if context has changed
// we don't do the bailout and we have to reuse existing props instead.
if (nextChildren === null) {
nextChildren = current && current.memoizedProps;
nextChildren = workInProgress.memoizedProps;
}
} else if (nextChildren === null || workInProgress.memoizedProps === nextChildren) {
return bailoutOnAlreadyFinishedWork(current, workInProgress);
}
reconcileChildren(current, workInProgress, nextChildren);
memoizeProps(workInProgress, nextChildren);
return workInProgress.child;
}

Expand All @@ -198,16 +203,20 @@ module.exports = function<T, P, I, TI, C, CX, CI>(
// Normally we can bail out on props equality but if context has changed
// we don't do the bailout and we have to reuse existing props instead.
if (nextProps === null) {
nextProps = current && current.memoizedProps;
nextProps = memoizedProps;
}
} else {
if (nextProps == null || memoizedProps === nextProps) {
return bailoutOnAlreadyFinishedWork(current, workInProgress);
}
// TODO: Disable this before release, since it is not part of the public API
// I use this for testing to compare the relative overhead of classes.
if (typeof fn.shouldComponentUpdate === 'function' &&
!fn.shouldComponentUpdate(memoizedProps, nextProps)) {
// Memoize props even if shouldComponentUpdate returns false
memoizeProps(workInProgress, nextProps);
return bailoutOnAlreadyFinishedWork(current, workInProgress);
}
} else if (nextProps === null || memoizedProps === nextProps || (
// TODO: Disable this before release, since it is not part of the public API
// I use this for testing to compare the relative overhead of classes.
memoizedProps !== null &&
typeof fn.shouldComponentUpdate === 'function' &&
!fn.shouldComponentUpdate(memoizedProps, nextProps)
)) {
return bailoutOnAlreadyFinishedWork(current, workInProgress);
}

var context = getMaskedContext(workInProgress);
Expand All @@ -221,6 +230,7 @@ module.exports = function<T, P, I, TI, C, CX, CI>(
nextChildren = fn(nextProps, context);
}
reconcileChildren(current, workInProgress, nextChildren);
memoizeProps(workInProgress, nextProps);
return workInProgress.child;
}

Expand Down Expand Up @@ -253,31 +263,23 @@ module.exports = function<T, P, I, TI, C, CX, CI>(
shouldUpdate : boolean,
hasContext : boolean,
) {
// Schedule side-effects

// Refs should update even if shouldComponentUpdate returns false
markRef(current, workInProgress);

if (shouldUpdate) {
workInProgress.effectTag |= Update;
} else {
// If an update was already in progress, we should schedule an Update
// effect even though we're bailing out, so that cWU/cDU are called.
if (current) {
const instance = current.stateNode;
if (instance.props !== current.memoizedProps ||
instance.state !== current.memoizedState) {
workInProgress.effectTag |= Update;
}
}
if (!shouldUpdate) {
return bailoutOnAlreadyFinishedWork(current, workInProgress);
}

// Rerender
const instance = workInProgress.stateNode;

// Rerender
ReactCurrentOwner.current = workInProgress;
const nextChildren = instance.render();
reconcileChildren(current, workInProgress, nextChildren);
// Memoize props and state using the values we just used to render.
// TODO: Restructure so we never read values from the instance.
memoizeState(workInProgress, instance.state);
memoizeProps(workInProgress, instance.props);

// The context might have changed so we need to recalculate it.
if (hasContext) {
Expand Down Expand Up @@ -316,7 +318,7 @@ module.exports = function<T, P, I, TI, C, CX, CI>(
}
const element = state.element;
reconcileChildren(current, workInProgress, element);
workInProgress.memoizedState = state;
memoizeState(workInProgress, state);
return workInProgress.child;
}
// If there is no update queue, that's a bailout because the root has no props.
Expand All @@ -333,7 +335,7 @@ module.exports = function<T, P, I, TI, C, CX, CI>(
// Normally we can bail out on props equality but if context has changed
// we don't do the bailout and we have to reuse existing props instead.
if (nextProps === null) {
nextProps = prevProps;
nextProps = memoizedProps;
if (!nextProps) {
throw new Error('We should always have pending or current props.');
}
Expand Down Expand Up @@ -398,6 +400,7 @@ module.exports = function<T, P, I, TI, C, CX, CI>(

// Reconcile the children and stash them for later work.
reconcileChildrenAtPriority(current, workInProgress, nextChildren, OffscreenPriority);
memoizeProps(workInProgress, nextProps);
workInProgress.child = current ? current.child : null;

if (!current) {
Expand All @@ -417,10 +420,22 @@ module.exports = function<T, P, I, TI, C, CX, CI>(
return null;
} else {
reconcileChildren(current, workInProgress, nextChildren);
memoizeProps(workInProgress, nextProps);
return workInProgress.child;
}
}

function updateHostText(current, workInProgress) {
let nextProps = workInProgress.pendingProps;
if (nextProps === null) {
nextProps = workInProgress.memoizedProps;
}
memoizeProps(workInProgress, nextProps);
// Nothing to do here. This is terminal. We'll do the completion step
// immediately after.
return null;
}

function mountIndeterminateComponent(current, workInProgress, priorityLevel) {
if (current) {
throw new Error('An indeterminate component should never have mounted.');
Expand Down Expand Up @@ -453,6 +468,7 @@ module.exports = function<T, P, I, TI, C, CX, CI>(
// Proceed under the assumption that this is a functional component
workInProgress.tag = FunctionalComponent;
reconcileChildren(current, workInProgress, value);
memoizeProps(workInProgress, props);
return workInProgress.child;
}
}
Expand All @@ -472,6 +488,7 @@ module.exports = function<T, P, I, TI, C, CX, CI>(
return bailoutOnAlreadyFinishedWork(current, workInProgress);
}
reconcileChildren(current, workInProgress, nextCoroutine.children);
memoizeProps(workInProgress, nextCoroutine);
// This doesn't take arbitrary time so we could synchronously just begin
// eagerly do the work of workInProgress.child as an optimization.
return workInProgress.child;
Expand Down Expand Up @@ -506,9 +523,11 @@ module.exports = function<T, P, I, TI, C, CX, CI>(
nextChildren,
priorityLevel
);
memoizeProps(workInProgress, nextChildren);
markChildAsProgressed(current, workInProgress, priorityLevel);
} else {
reconcileChildren(current, workInProgress, nextChildren);
memoizeProps(workInProgress, nextChildren);
}
return workInProgress.child;
}
Expand Down Expand Up @@ -575,6 +594,18 @@ module.exports = function<T, P, I, TI, C, CX, CI>(
return null;
}

function memoizeProps(workInProgress : Fiber, nextProps : any) {
workInProgress.memoizedProps = nextProps;
// Reset the pending props
workInProgress.pendingProps = null;
}

function memoizeState(workInProgress : Fiber, nextState : any) {
workInProgress.memoizedState = nextState;
// Don't reset the updateQueue, in case there are pending updates. Resetting
// is handled by beginUpdateQueue.
}

function beginWork(current : ?Fiber, workInProgress : Fiber, priorityLevel : PriorityLevel) : ?Fiber {
if (workInProgress.pendingWorkPriority === NoWork ||
workInProgress.pendingWorkPriority > priorityLevel) {
Expand Down Expand Up @@ -608,9 +639,7 @@ module.exports = function<T, P, I, TI, C, CX, CI>(
case HostComponent:
return updateHostComponent(current, workInProgress);
case HostText:
// Nothing to do here. This is terminal. We'll do the completion step
// immediately after.
return null;
return updateHostText(current, workInProgress);
case CoroutineHandlerPhase:
// This is a restart. Reset the tag to the initial phase.
workInProgress.tag = CoroutineComponent;
Expand Down Expand Up @@ -652,6 +681,14 @@ module.exports = function<T, P, I, TI, C, CX, CI>(
// Unmount the current children as if the component rendered null
const nextChildren = null;
reconcileChildren(current, workInProgress, nextChildren);

if (workInProgress.tag === ClassComponent) {
const instance = workInProgress.stateNode;
workInProgress.memoizedProps = instance.props;
workInProgress.memoizedState = instance.state;
workInProgress.pendingProps = null;
}

return workInProgress.child;
}

Expand Down
Loading

0 comments on commit ef96a54

Please sign in to comment.