From 05f3ecc3eae7a6c10109f48548c001c28f4cb480 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Tue, 7 Nov 2017 16:52:48 +0000 Subject: [PATCH] Performance tool: Warn when interrupting an in-progress tree (#11480) * Performance tool: Warn when interrupting an in-progress tree * Include the name of the component that caused the interruption --- .../src/ReactDebugFiberPerf.js | 19 +++++++++---- .../src/ReactFiberScheduler.js | 19 ++++++++----- .../__tests__/ReactIncrementalPerf-test.js | 14 ++++++++++ .../ReactIncrementalPerf-test.js.snap | 27 ++++++++++++++++--- 4 files changed, 64 insertions(+), 15 deletions(-) diff --git a/packages/react-reconciler/src/ReactDebugFiberPerf.js b/packages/react-reconciler/src/ReactDebugFiberPerf.js index ad04bc9cec..625dcda11b 100644 --- a/packages/react-reconciler/src/ReactDebugFiberPerf.js +++ b/packages/react-reconciler/src/ReactDebugFiberPerf.js @@ -318,8 +318,9 @@ export function stopPhaseTimer(): void { } } -export function startWorkLoopTimer(): void { +export function startWorkLoopTimer(nextUnitOfWork: Fiber | null): void { if (enableUserTimingAPI) { + currentFiber = nextUnitOfWork; if (!supportsUserTiming) { return; } @@ -332,14 +333,22 @@ export function startWorkLoopTimer(): void { } } -export function stopWorkLoopTimer(): void { +export function stopWorkLoopTimer(interruptedBy: Fiber | null): void { if (enableUserTimingAPI) { if (!supportsUserTiming) { return; } - const warning = commitCountInCurrentWorkLoop > 1 - ? 'There were cascading updates' - : null; + let warning = null; + if (interruptedBy !== null) { + if (interruptedBy.tag === HostRoot) { + warning = 'A top-level update interrupted the previous render'; + } else { + const componentName = getComponentName(interruptedBy) || 'Unknown'; + warning = `An update to ${componentName} interrupted the previous render`; + } + } else if (commitCountInCurrentWorkLoop > 1) { + warning = 'There were cascading updates'; + } commitCountInCurrentWorkLoop = 0; // Pause any measurements until the next loop. pauseTimers(); diff --git a/packages/react-reconciler/src/ReactFiberScheduler.js b/packages/react-reconciler/src/ReactFiberScheduler.js index 19dfef14aa..7bdc89c707 100644 --- a/packages/react-reconciler/src/ReactFiberScheduler.js +++ b/packages/react-reconciler/src/ReactFiberScheduler.js @@ -216,6 +216,9 @@ export default function( let isCommitting: boolean = false; let isUnmounting: boolean = false; + // Used for performance tracking. + let interruptedBy: Fiber | null = null; + function resetContextStack() { // Reset the stack reset(); @@ -752,8 +755,6 @@ export default function( root: FiberRoot, expirationTime: ExpirationTime, ): Fiber | null { - startWorkLoopTimer(); - invariant( !isWorking, 'renderRoot was called recursively. This error is likely caused ' + @@ -772,7 +773,7 @@ export default function( expirationTime !== nextRenderExpirationTime || nextUnitOfWork === null ) { - // This is a restart. Reset the stack. + // Reset the stack and start working from the root. resetContextStack(); nextRoot = root; nextRenderExpirationTime = expirationTime; @@ -783,6 +784,8 @@ export default function( ); } + startWorkLoopTimer(nextUnitOfWork); + let didError = false; let error = null; if (__DEV__) { @@ -865,12 +868,12 @@ export default function( const uncaughtError = firstUncaughtError; // We're done performing work. Time to clean up. + stopWorkLoopTimer(interruptedBy); + interruptedBy = null; isWorking = false; didFatal = false; firstUncaughtError = null; - stopWorkLoopTimer(); - if (uncaughtError !== null) { onUncaughtError(uncaughtError); } @@ -1198,7 +1201,11 @@ export default function( root === nextRoot && expirationTime <= nextRenderExpirationTime ) { - // This is an interruption. Restart the root from the top. + // Restart the root from the top. + if (nextUnitOfWork !== null) { + // This is an interruption. (Used for performance tracking.) + interruptedBy = fiber; + } nextRoot = null; nextUnitOfWork = null; nextRenderExpirationTime = NoWork; diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalPerf-test.js b/packages/react-reconciler/src/__tests__/ReactIncrementalPerf-test.js index 5b699c9414..f690be2b08 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalPerf-test.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalPerf-test.js @@ -498,4 +498,18 @@ describe('ReactDebugFiberPerf', () => { }); expect(getFlameChart()).toMatchSnapshot(); }); + + it('warns if an in-progress update is interrupted', () => { + function Foo() { + return ; + } + + ReactNoop.render(); + ReactNoop.flushUnitsOfWork(2); + ReactNoop.flushSync(() => { + ReactNoop.render(); + }); + ReactNoop.flush(); + expect(getFlameChart()).toMatchSnapshot(); + }); }); diff --git a/packages/react-reconciler/src/__tests__/__snapshots__/ReactIncrementalPerf-test.js.snap b/packages/react-reconciler/src/__tests__/__snapshots__/ReactIncrementalPerf-test.js.snap index 4815a22fc0..0bd35bd701 100644 --- a/packages/react-reconciler/src/__tests__/__snapshots__/ReactIncrementalPerf-test.js.snap +++ b/packages/react-reconciler/src/__tests__/__snapshots__/ReactIncrementalPerf-test.js.snap @@ -243,6 +243,17 @@ exports[`ReactDebugFiberPerf skips parents during setState 1`] = ` " `; +exports[`ReactDebugFiberPerf supports portals 1`] = ` +"⚛ (React Tree Reconciliation) + ⚛ Parent [mount] + ⚛ Child [mount] + +⚛ (Committing Changes) + ⚛ (Committing Host Effects: 2 Total) + ⚛ (Calling Lifecycle Methods: 0 Total) +" +`; + exports[`ReactDebugFiberPerf supports returns 1`] = ` "⚛ (React Tree Reconciliation) ⚛ App [mount] @@ -260,14 +271,22 @@ exports[`ReactDebugFiberPerf supports returns 1`] = ` " `; -exports[`ReactDebugFiberPerf supports portals 1`] = ` +exports[`ReactDebugFiberPerf warns if an in-progress update is interrupted 1`] = ` "⚛ (React Tree Reconciliation) - ⚛ Parent [mount] - ⚛ Child [mount] + ⚛ Foo [mount] + +⛔ (React Tree Reconciliation) Warning: A top-level update interrupted the previous render + ⚛ Foo [mount] ⚛ (Committing Changes) - ⚛ (Committing Host Effects: 2 Total) + ⚛ (Committing Host Effects: 1 Total) ⚛ (Calling Lifecycle Methods: 0 Total) + +⚛ (React Tree Reconciliation) + +⚛ (Committing Changes) + ⚛ (Committing Host Effects: 1 Total) + ⚛ (Calling Lifecycle Methods: 1 Total) " `;