Skip to content

Update user timing to record when we are about to commit #12384

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

Merged
merged 6 commits into from
Apr 2, 2018
Merged

Update user timing to record when we are about to commit #12384

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
fe884f1
Update user timing to record when we are about to commit
flarnie Mar 15, 2018
516f741
Ran prettier
flarnie Mar 15, 2018
4ed4a45
Fixes in response to code review
flarnie Mar 16, 2018
7eaba37
Update snapshot tests
flarnie Mar 16, 2018
5875830
Move isWorking assignment out of branches to top
flarnie Mar 28, 2018
477757c
Stricter type for stopWorkLoopTimer args
flarnie Apr 2, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Fixes in response to code review
  • Loading branch information
@flarnie
flarnie committed Apr 2, 2018
commit 4ed4a45b726d79b4bba372a22ab858845c0d54da
2 changes: 1 addition & 1 deletion packages/react-reconciler/src/ReactDebugFiberPerf.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -393,7 +393,7 @@ export function stopWorkLoopTimer(
commitCountInCurrentWorkLoop = 0;
let label = didCompleteRoot
? '(React Tree Reconciliation: Completed Root)'
: '(React Tree Reconciliation)';
: '(React Tree Reconciliation: Yielded)';
// Pause any measurements until the next loop.
pauseTimers();
endMark(label, '(React Tree Reconciliation)', warning);
Expand Down
18 changes: 14 additions & 4 deletions packages/react-reconciler/src/ReactFiberScheduler.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -958,13 +958,13 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
} while (true);

// We're done performing work. Time to clean up.
const didCompleteRoot = nextUnitOfWork === null && isRootReadyForCommit;
stopWorkLoopTimer(interruptedBy, didCompleteRoot);
interruptedBy = null;
isWorking = false;
let didCompleteRoot = null;

// Yield back to main thread.
if (didFatal) {
stopWorkLoopTimer(interruptedBy, didCompleteRoot);
interruptedBy = null;
isWorking = false;
// There was a fatal error.
if (__DEV__) {
stack.resetStackAfterFatalErrorInDev();
Expand All @@ -973,19 +973,29 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
} else if (nextUnitOfWork === null) {
// We reached the root.
if (isRootReadyForCommit) {
didCompleteRoot = true;
stopWorkLoopTimer(interruptedBy, didCompleteRoot);
interruptedBy = null;
isWorking = false;
// The root successfully completed. It's ready for commit.
root.pendingCommitExpirationTime = expirationTime;
const finishedWork = root.current.alternate;
return finishedWork;
} else {
// The root did not complete.
stopWorkLoopTimer(interruptedBy, didCompleteRoot);
interruptedBy = null;
isWorking = false;
invariant(
false,
'Expired work should have completed. This error is likely caused ' +
'by a bug in React. Please file an issue.',
);
}
} else {
stopWorkLoopTimer(interruptedBy, didCompleteRoot);
interruptedBy = null;
isWorking = false;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Sorry, another nit: can we move isWorking back out? :) It's not related to the work timer and it's the same in every branch.

// There's more work to do, but we ran out of time. Yield back to
// the renderer.
return null;
Expand Down