Skip to content

Commit

Permalink
Deterministic updates
Browse files Browse the repository at this point in the history
High priority updates typically require less work to render than
low priority ones. It's beneficial to flush those first, in their own
batch, before working on more expensive low priority ones. We do this
even if a high priority is scheduled after a low priority one.

However, we don't want this reordering of updates to affect the terminal
state. State should be deterministic: once all work has been flushed,
the final state should be the same regardless of how they were
scheduled.

To get both properties, we store updates on the queue in insertion
order instead of priority order (always append). Then, when processing
the queue, we skip over updates with insufficient priority. Instead of
removing updates from the queue right after processing them, we only
remove them if there are no unprocessed updates before it in the list.

This means that updates may be processed more than once.

As a bonus, the new implementation is simpler and requires less code.
  • Loading branch information
acdlite committed Oct 8, 2017
1 parent 75e2a6e commit d70d52e
Show file tree
Hide file tree
Showing 15 changed files with 343 additions and 322 deletions.
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@
"glob-stream": "^6.1.0",
"gzip-js": "~0.3.2",
"gzip-size": "^3.0.0",
"jasmine-check": "^1.0.0-rc.0",
"jest": "20.1.0-delta.1",
"jest-config": "20.1.0-delta.1",
"jest-jasmine2": "20.1.0-delta.1",
Expand Down
2 changes: 2 additions & 0 deletions scripts/jest/test-framework-setup.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,4 +68,6 @@ if (process.env.REACT_CLASS_EQUIVALENCE_TEST) {
return expectation;
};
global.expectDev = expectDev;

require('jasmine-check').install();
}
4 changes: 2 additions & 2 deletions src/renderers/dom/fiber/__tests__/ReactDOMFiberAsync-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -284,8 +284,8 @@ describe('ReactDOMFiberAsync', () => {

// Flush the async updates
jest.runAllTimers();
expect(container.textContent).toEqual('BCAD');
expect(ops).toEqual(['BC', 'BCAD']);
expect(container.textContent).toEqual('ABCD');
expect(ops).toEqual(['BC', 'ABCD']);
});
});
});
9 changes: 9 additions & 0 deletions src/renderers/noop/ReactNoopEntry.js
Original file line number Diff line number Diff line change
Expand Up @@ -296,9 +296,18 @@ var ReactNoop = {
const root = NoopRenderer.createContainer(container);
roots.set(rootID, root);
return {
render(children: any) {
const work = NoopRenderer.updateRoot(children, root, null);
work.then(() => work.commit());
},
prerender(children: any) {
return NoopRenderer.updateRoot(children, root, null);
},
unmount() {
roots.delete(rootID);
const work = NoopRenderer.updateRoot(null, root, null);
work.then(() => work.commit());
},
getChildren() {
return ReactNoop.getChildren(rootID);
},
Expand Down
3 changes: 1 addition & 2 deletions src/renderers/shared/fiber/ReactFiberBeginWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,6 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
workInProgress,
updateQueue,
null,
prevState,
null,
renderExpirationTime,
);
Expand All @@ -349,7 +348,7 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
resetHydrationState();
return bailoutOnAlreadyFinishedWork(current, workInProgress);
}
const element = state.element;
const element = state !== null ? state.element : null;
if (
root.hydrate &&
(current === null || current.child === null) &&
Expand Down
4 changes: 1 addition & 3 deletions src/renderers/shared/fiber/ReactFiberClassComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,7 @@ module.exports = function(
const unmaskedContext = getUnmaskedContext(workInProgress);

instance.props = props;
instance.state = state;
instance.state = workInProgress.memoizedState = state;
instance.refs = emptyObject;
instance.context = getMaskedContext(workInProgress, unmaskedContext);

Expand All @@ -422,7 +422,6 @@ module.exports = function(
workInProgress,
updateQueue,
instance,
state,
props,
renderExpirationTime,
);
Expand Down Expand Up @@ -589,7 +588,6 @@ module.exports = function(
workInProgress,
workInProgress.updateQueue,
instance,
oldState,
newProps,
renderExpirationTime,
);
Expand Down
45 changes: 19 additions & 26 deletions src/renderers/shared/fiber/ReactFiberCommitWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,7 @@ var {
clearCaughtError,
} = require('ReactErrorUtils');

var {
Placement,
Update,
Callback,
ContentReset,
} = require('ReactTypeOfSideEffect');
var {Placement, Update, ContentReset} = require('ReactTypeOfSideEffect');

var invariant = require('fbjs/lib/invariant');

Expand Down Expand Up @@ -487,16 +482,26 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
}
}

function commitCallbacks(callbackList, context) {
for (let i = 0; i < callbackList.length; i++) {
const callback = callbackList[i];
function commitCallbacks(updateQueue, context) {
let callbackNode = updateQueue.firstCallback;
// Reset the callback list before calling them in case something throws.
updateQueue.firstCallback = updateQueue.lastCallback = null;

while (callbackNode !== null) {
const callback = callbackNode.callback;
// Remove this callback from the update object in case it's still part
// of the queue, so that we don't call it again.
callbackNode.callback = null;
invariant(
typeof callback === 'function',
'Invalid argument passed as callback. Expected a function. Instead ' +
'received: %s',
callback,
);
callback.call(context);
const nextCallback = callbackNode.nextCallback;
callbackNode.nextCallback = null;
callbackNode = nextCallback;
}
}

Expand Down Expand Up @@ -529,31 +534,19 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
}
}
}
if (
finishedWork.effectTag & Callback &&
finishedWork.updateQueue !== null
) {
const updateQueue = finishedWork.updateQueue;
if (updateQueue.callbackList !== null) {
// Set the list to null to make sure they don't get called more than once.
const callbackList = updateQueue.callbackList;
updateQueue.callbackList = null;
commitCallbacks(callbackList, instance);
}
const updateQueue = finishedWork.updateQueue;
if (updateQueue !== null) {
commitCallbacks(updateQueue, instance);
}
return;
}
case HostRoot: {
const updateQueue = finishedWork.updateQueue;
if (updateQueue !== null && updateQueue.callbackList !== null) {
// Set the list to null to make sure they don't get called more
// than once.
const callbackList = updateQueue.callbackList;
updateQueue.callbackList = null;
if (updateQueue !== null) {
const instance = finishedWork.child !== null
? finishedWork.child.stateNode
: null;
commitCallbacks(callbackList, instance);
commitCallbacks(updateQueue, instance);
}
return;
}
Expand Down
33 changes: 6 additions & 27 deletions src/renderers/shared/fiber/ReactFiberReconciler.js
Original file line number Diff line number Diff line change
Expand Up @@ -286,43 +286,22 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
callback,
);
}
const isTopLevelUnmount = nextState.element === null;
const update = {
priorityLevel,
expirationTime,
partialState: nextState,
callback,
isReplace: false,
isForced: false,
isTopLevelUnmount,
nextCallback: null,
next: null,
};
const update2 = insertUpdateIntoFiber(current, update, currentTime);

if (isTopLevelUnmount) {
// TODO: Redesign the top-level mount/update/unmount API to avoid this
// special case.
const queue1 = current.updateQueue;
const queue2 = current.alternate !== null
? current.alternate.updateQueue
: null;

// Drop all updates that are lower-priority, so that the tree is not
// remounted. We need to do this for both queues.
if (queue1 !== null && update.next !== null) {
update.next = null;
queue1.last = update;
}
if (queue2 !== null && update2 !== null && update2.next !== null) {
update2.next = null;
queue2.last = update;
}
}
insertUpdateIntoFiber(current, update, currentTime);

if (isPrerender) {
// Block the root from committing at this expiration time.
if (root.topLevelBlockers === null) {
root.topLevelBlockers = createUpdateQueue();
root.topLevelBlockers = createUpdateQueue(null);
}
const block = {
priorityLevel: null,
Expand All @@ -331,7 +310,7 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
callback: null,
isReplace: false,
isForced: false,
isTopLevelUnmount: false,
nextCallback: null,
next: null,
};
insertUpdateIntoQueue(root.topLevelBlockers, block, currentTime);
Expand All @@ -352,7 +331,7 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
if (topLevelBlockers === null) {
return;
}
processUpdateQueue(topLevelBlockers, null, null, null, expirationTime);
processUpdateQueue(topLevelBlockers, null, null, expirationTime);
expireWork(root, expirationTime);
};
WorkNode.prototype.then = function(callback) {
Expand Down Expand Up @@ -403,7 +382,7 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(

let completionCallbacks = container.completionCallbacks;
if (completionCallbacks === null) {
completionCallbacks = createUpdateQueue();
completionCallbacks = createUpdateQueue(null);
}

return new WorkNode(container, expirationTime);
Expand Down
30 changes: 16 additions & 14 deletions src/renderers/shared/fiber/ReactFiberScheduler.js
Original file line number Diff line number Diff line change
Expand Up @@ -386,21 +386,23 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
// the end of the current batch.
const completionCallbacks = root.completionCallbacks;
if (completionCallbacks !== null) {
processUpdateQueue(completionCallbacks, null, null, null, completedAt);
const callbackList = completionCallbacks.callbackList;
if (callbackList !== null) {
// Add new callbacks to list of completion callbacks
processUpdateQueue(completionCallbacks, null, null, completedAt);
// Add new callbacks to list of completion callbacks
let callbackNode = completionCallbacks.firstCallback;
completionCallbacks.firstCallback = completionCallbacks.lastCallback = null;
while (callbackNode !== null) {
const callback: () => mixed = (callbackNode.callback: any);
// Remove this callback from the update object in case it's still part
// of the queue, so that we don't call it again.
callbackNode.callback = null;
if (rootCompletionCallbackList === null) {
rootCompletionCallbackList = callbackList;
rootCompletionCallbackList = [callback];
} else {
for (let i = 0; i < callbackList.length; i++) {
rootCompletionCallbackList.push(callbackList[i]);
}
}
completionCallbacks.callbackList = null;
if (completionCallbacks.first === null) {
root.completionCallbacks = null;
rootCompletionCallbackList.push(callback);
}
const nextCallback = callbackNode.nextCallback;
callbackNode.nextCallback = null;
callbackNode = nextCallback;
}
}
}
Expand Down Expand Up @@ -1636,12 +1638,12 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
callback,
isReplace: false,
isForced: false,
isTopLevelUnmount: false,
nextCallback: null,
next: null,
};
const currentTime = recalculateCurrentTime();
if (root.completionCallbacks === null) {
root.completionCallbacks = createUpdateQueue();
root.completionCallbacks = createUpdateQueue(null);
}
insertUpdateIntoQueue(root.completionCallbacks, update, currentTime);
if (expirationTime === root.completedAt) {
Expand Down
Loading

0 comments on commit d70d52e

Please sign in to comment.