[Transition Tracing] Add Support for Multiple Transitions on Root #24732

lunaruan · 2022-06-15T23:04:50Z

We can think of transitions on the root as a bunch of tracing markers. Therefore, we can map each transition to a map of pending suspense boundaries. When a transition's pending suspense boundary map is empty, we know that it's complete. This PR:

Combines the pendingSuspenseBoundaries and transitions into one incompleteTransitions object. This object is a map from a transition to a map of pendingSuspenseBoundaries
Refactored code to make it so that every transition has its own pendingSuspenseBoundaries map rather than sharing just one.
Moves the transition complete callback to the root. Alternatively, we can also keep a map of pendingSuspenseBoundaries to transitions on the Offscreen marker, but it's simpler to just call the transition complete callback on the root instead. We also only do this if there are transitions pending, so it shouldn't make too big of a difference

sizebot · 2022-06-15T23:55:46Z

Comparing: fcd720d...dd962d3

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name	+/-	Base	Current	+/- gzip	Base gzip	Current gzip
oss-stable/react-dom/cjs/react-dom.production.min.js	=	131.76 kB	131.64 kB	=	42.31 kB	42.30 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js	=	137.03 kB	136.92 kB	=	43.90 kB	43.89 kB
facebook-www/ReactDOM-prod.classic.js	=	440.91 kB	440.67 kB	=	80.63 kB	80.59 kB
facebook-www/ReactDOM-prod.modern.js	=	426.21 kB	425.98 kB	=	78.44 kB	78.41 kB
facebook-www/ReactDOMForked-prod.classic.js	=	441.61 kB	441.38 kB	=	80.85 kB	80.81 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against dd962d3

lunaruan · 2022-06-16T21:02:02Z

packages/react-reconciler/src/ReactFiberCommitWork.old.js

-    const rootPendingBoundaries = rootState.pendingSuspenseBoundaries;
-    const rootTransitions = rootState.transitions;
-
+    const suspenseBoundaries = (offscreenFiber.updateQueue: any).markers;


Rename this

lunaruan · 2022-06-16T21:25:27Z

packages/react-reconciler/src/ReactFiberCommitWork.new.js

+              queue.markers = new Set();
+            }
+            if (transitions !== null && prevTransitions === null) {
+              finishedWork.memoizedState.transitions = prevTransitions = new Set();


Move markers and transitions to the instance

Also move everything to root

lunaruan · 2022-06-16T21:26:19Z

packages/react-reconciler/src/__tests__/ReactTransitionTracing-test.js

@@ -279,4 +279,107 @@ describe('ReactInteractionTracing', () => {
      ]);
    });
  });
+
+  // @gate enableTransitionTracing
+  it('should correctly trace multiple root interactions', async () => {


Write another test

lunaruan · 2022-06-16T21:26:55Z

packages/react-reconciler/src/ReactFiberCommitWork.new.js

+              queue.markers = new Set();
+            }
+            if (transitions !== null && prevTransitions === null) {
+              finishedWork.memoizedState.transitions = prevTransitions = new Set();


Also move everything to root

lunaruan · 2022-06-17T18:34:05Z

packages/react-reconciler/src/ReactFiberCommitWork.new.js

-        const pendingTransitions = state.transitions;
-        const pendingSuspenseBoundaries = state.pendingSuspenseBoundaries;
-
+        let incompleteTransitions = state.incompleteTransitions;


Will move this code to the HostRoot in a separate PR

acdlite · 2022-06-16T20:56:13Z

packages/react-reconciler/src/ReactFiberBeginWork.new.js

@@ -2107,6 +2106,7 @@ function updateSuspenseComponent(current, workInProgress, renderLanes) {
        if (currentTransitions !== null) {
          const primaryChildUpdateQueue: OffscreenQueue = {
            transitions: currentTransitions,
+            markers: null,


Don't need this yet

acdlite · 2022-06-17T19:16:09Z

packages/react-reconciler/src/ReactFiberBeginWork.new.js

-        // be added to the stack to get added to any Offscreen/suspense children
-        transitions = workInProgress.memoizedState.transitions;
+      if (enableTransitionTracing) {
+        if (workInProgress.stateNode.transitions !== null) {


You don't need to check if it's null, right? It can just pass through

acdlite · 2022-06-17T19:16:58Z

packages/react-reconciler/src/ReactFiberCommitWork.new.js

+            // and sets in the commit phase as we need to use them. We only
+            // instantiate them in the fallback phase on an as needed basis
+            if (rootMemoizedState.incompleteTransitions === null) {
+              rootMemoizedState.incompleteTransitions = rootIncompleteTransitions = new Map();


Could you add a TODO here to move this to the FiberRoot?

Summary: This sync includes the following changes: - **[c1f5884ff](facebook/react@c1f5884ff )**: Add missing null checks to OffscreenInstance code ([#24846](facebook/react#24846)) //<Andrew Clark>// - **[4cd788aef](facebook/react@4cd788aef )**: Revert "Revert [Transition Tracing] Refactor Transition Tracing Root Code" ([#24830](facebook/react#24830)) //<Luna Ruan>// - **[e61fd91f5](facebook/react@e61fd91f5 )**: Revert "[Transition Tracing] Refactor Transition Tracing Root Code ([#24766](facebook/react#24766))" ([#24829](facebook/react#24829)) //<Andrew Clark>// - **[401296310](facebook/react@401296310 )**: [Transition Tracing] Refactor Transition Tracing Root Code ([#24766](facebook/react#24766)) //<Luna Ruan>// - **[185932902](facebook/react@185932902 )**: Track nearest Suspense handler on stack ([#24585](facebook/react#24585)) //<Andrew Clark>// - **[a7b192e0f](facebook/react@a7b192e0f )**: Add test gate alias for Offscreen ([#24749](facebook/react#24749)) //<Andrew Clark>// - **[6b6cf8311](facebook/react@6b6cf8311 )**: Land forked reconciler changes ([#24817](facebook/react#24817)) //<Andrew Clark>// - **[d1432ba93](facebook/react@d1432ba93 )**: [Transition Tracing] Fix excess calls to the transition start callback ([#24806](facebook/react#24806)) //<Luna Ruan>// - **[88574c1b8](facebook/react@88574c1b8 )**: Fix enableTransitionTracing flag ([#24801](facebook/react#24801)) //<Luna Ruan>// - **[a4bed4696](facebook/react@a4bed4696 )**: [Transition Tracing] Add Tracing Markers ([#24686](facebook/react#24686)) //<Luna Ruan>// - **[167853026](facebook/react@167853026 )**: fix hydration warning suppression in text comparisons ([#24784](facebook/react#24784)) //<Josh Story>// - **[9abe745aa](facebook/react@9abe745aa )**: [DevTools][Timeline Profiler] Component Stacks Backend ([#24776](facebook/react#24776)) //<Luna Ruan>// - **[cf665c4b7](facebook/react@cf665c4b7 )**: [DevTools] Refactor incompleteTransitions field from Root Fiber memoized state to FiberRoot ([#24765](facebook/react#24765)) //<Luna Ruan>// - **[56389e81f](facebook/react@56389e81f )**: Abort Flight ([#24754](facebook/react#24754)) //<Sebastian Markbåge>// - **[0f216ae31](facebook/react@0f216ae31 )**: Add entry points for "static" server rendering passes ([#24752](facebook/react#24752)) //<Sebastian Markbåge>// - **[f796fa13a](facebook/react@f796fa13a )**: Rename Segment to Task in Flight ([#24753](facebook/react#24753)) //<Sebastian Markbåge>// - **[0f0aca3ab](facebook/react@0f0aca3ab )**: Aborting early should not infinitely suspend ([#24751](facebook/react#24751)) //<Sebastian Markbåge>// - **[12a738f1a](facebook/react@12a738f1a )**: [Transition Tracing] Add Support for Multiple Transitions on Root ([#24732](facebook/react#24732)) //<Luna Ruan>// - **[72ebc703a](facebook/react@72ebc703a )**: [DevTools] fix useDeferredValue to match reconciler change ([#24742](facebook/react#24742)) //<Mengdi Chen>// - **[7cf9f5e03](facebook/react@7cf9f5e03 )**: Extra space ([#24612](facebook/react#24612)) //<Kerim Büyükakyüz>// Changelog: [General][Changed] - React Native sync for revisions 229c86a...c1f5884 Reviewed By: mdvacca, GijsWeterings Differential Revision: D38904311 fbshipit-source-id: 1e30bc420c30ec7a0c0073fc92a706afef4b3340

Summary: This sync includes the following changes: - **[c1f5884ff](facebook/react@c1f5884ff )**: Add missing null checks to OffscreenInstance code ([facebook#24846](facebook/react#24846)) //<Andrew Clark>// - **[4cd788aef](facebook/react@4cd788aef )**: Revert "Revert [Transition Tracing] Refactor Transition Tracing Root Code" ([facebook#24830](facebook/react#24830)) //<Luna Ruan>// - **[e61fd91f5](facebook/react@e61fd91f5 )**: Revert "[Transition Tracing] Refactor Transition Tracing Root Code ([facebook#24766](facebook/react#24766))" ([facebook#24829](facebook/react#24829)) //<Andrew Clark>// - **[401296310](facebook/react@401296310 )**: [Transition Tracing] Refactor Transition Tracing Root Code ([facebook#24766](facebook/react#24766)) //<Luna Ruan>// - **[185932902](facebook/react@185932902 )**: Track nearest Suspense handler on stack ([facebook#24585](facebook/react#24585)) //<Andrew Clark>// - **[a7b192e0f](facebook/react@a7b192e0f )**: Add test gate alias for Offscreen ([facebook#24749](facebook/react#24749)) //<Andrew Clark>// - **[6b6cf8311](facebook/react@6b6cf8311 )**: Land forked reconciler changes ([facebook#24817](facebook/react#24817)) //<Andrew Clark>// - **[d1432ba93](facebook/react@d1432ba93 )**: [Transition Tracing] Fix excess calls to the transition start callback ([facebook#24806](facebook/react#24806)) //<Luna Ruan>// - **[88574c1b8](facebook/react@88574c1b8 )**: Fix enableTransitionTracing flag ([facebook#24801](facebook/react#24801)) //<Luna Ruan>// - **[a4bed4696](facebook/react@a4bed4696 )**: [Transition Tracing] Add Tracing Markers ([facebook#24686](facebook/react#24686)) //<Luna Ruan>// - **[167853026](facebook/react@167853026 )**: fix hydration warning suppression in text comparisons ([facebook#24784](facebook/react#24784)) //<Josh Story>// - **[9abe745aa](facebook/react@9abe745aa )**: [DevTools][Timeline Profiler] Component Stacks Backend ([facebook#24776](facebook/react#24776)) //<Luna Ruan>// - **[cf665c4b7](facebook/react@cf665c4b7 )**: [DevTools] Refactor incompleteTransitions field from Root Fiber memoized state to FiberRoot ([facebook#24765](facebook/react#24765)) //<Luna Ruan>// - **[56389e81f](facebook/react@56389e81f )**: Abort Flight ([facebook#24754](facebook/react#24754)) //<Sebastian Markbåge>// - **[0f216ae31](facebook/react@0f216ae31 )**: Add entry points for "static" server rendering passes ([facebook#24752](facebook/react#24752)) //<Sebastian Markbåge>// - **[f796fa13a](facebook/react@f796fa13a )**: Rename Segment to Task in Flight ([facebook#24753](facebook/react#24753)) //<Sebastian Markbåge>// - **[0f0aca3ab](facebook/react@0f0aca3ab )**: Aborting early should not infinitely suspend ([facebook#24751](facebook/react#24751)) //<Sebastian Markbåge>// - **[12a738f1a](facebook/react@12a738f1a )**: [Transition Tracing] Add Support for Multiple Transitions on Root ([facebook#24732](facebook/react#24732)) //<Luna Ruan>// - **[72ebc703a](facebook/react@72ebc703a )**: [DevTools] fix useDeferredValue to match reconciler change ([facebook#24742](facebook/react#24742)) //<Mengdi Chen>// - **[7cf9f5e03](facebook/react@7cf9f5e03 )**: Extra space ([facebook#24612](facebook/react#24612)) //<Kerim Büyükakyüz>// Changelog: [General][Changed] - React Native sync for revisions 229c86a...c1f5884 Reviewed By: mdvacca, GijsWeterings Differential Revision: D38904311 fbshipit-source-id: 1e30bc420c30ec7a0c0073fc92a706afef4b3340

lunaruan requested a review from acdlite June 15, 2022 23:04

facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Jun 15, 2022

lunaruan force-pushed the multiple_roots branch from faba079 to a51ea81 Compare June 15, 2022 23:05

lunaruan force-pushed the multiple_roots branch from a51ea81 to da4ed39 Compare June 16, 2022 01:59

lunaruan commented Jun 16, 2022

View reviewed changes

lunaruan force-pushed the multiple_roots branch from da4ed39 to 82cc491 Compare June 17, 2022 18:32

lunaruan commented Jun 17, 2022

View reviewed changes

lunaruan force-pushed the multiple_roots branch from 82cc491 to 50f7435 Compare June 17, 2022 19:00

lunaruan added 2 commits June 17, 2022 15:08

add support for multiple roots

74debbf

old

e8bf4f6

lunaruan force-pushed the multiple_roots branch from 50f7435 to e8bf4f6 Compare June 17, 2022 19:08

acdlite approved these changes Jun 17, 2022

View reviewed changes

lunaruan added 2 commits June 17, 2022 16:27

fixed nits

0d186bb

fixed nits old

dd962d3

lunaruan merged commit 12a738f into facebook:main Jun 17, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Transition Tracing] Add Support for Multiple Transitions on Root #24732

[Transition Tracing] Add Support for Multiple Transitions on Root #24732

lunaruan commented Jun 15, 2022

sizebot commented Jun 15, 2022 •

edited

Loading

lunaruan Jun 16, 2022

lunaruan Jun 16, 2022

lunaruan Jun 16, 2022

lunaruan Jun 16, 2022

lunaruan Jun 16, 2022

lunaruan Jun 17, 2022

acdlite Jun 16, 2022

acdlite Jun 17, 2022

acdlite Jun 17, 2022

[Transition Tracing] Add Support for Multiple Transitions on Root #24732

[Transition Tracing] Add Support for Multiple Transitions on Root #24732

Conversation

lunaruan commented Jun 15, 2022

sizebot commented Jun 15, 2022 • edited Loading

Critical size changes

Significant size changes

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sizebot commented Jun 15, 2022 •

edited

Loading