Skip to content
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.

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

[WIP][Transition Tracing] Transition Tracing API #23103

Closed
wants to merge 13 commits into from

Conversation

lunaruan
Copy link
Contributor

@lunaruan lunaruan commented Jan 12, 2022

Would love some preliminary feedback on this!


Setup

  • Create enableTransitionTracing feature flag
  • Create TracingMarker component boilerplate
  • Add transition callback functions as arguments to the root
  • Add currentPendingTransitionCallbacks global on ReactFiberWorkLoop of type:

Array

  • Create a function on the ReactDOMHostConfig to get start time as detailed above
  • Create a function on the ReactDOMHostConfig to get end time and process the callbacks

Render Phase

  • Add a transitionName to start transition, store the transition start time and name in the batch config, and pass it to the root on render

@lunaruan lunaruan requested review from acdlite and bvaughn January 12, 2022 20:25
@facebook-github-bot facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Jan 12, 2022
@sizebot
Copy link

sizebot commented Jan 12, 2022

Comparing: 1fb0d06...9ad9220

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 = 130.59 kB 130.60 kB +0.01% 41.86 kB 41.86 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js = 135.77 kB 135.78 kB = 43.41 kB 43.42 kB
facebook-www/ReactDOM-prod.classic.js = 432.26 kB 432.29 kB +0.01% 79.27 kB 79.28 kB
facebook-www/ReactDOM-prod.modern.js = 422.15 kB 422.18 kB +0.01% 77.82 kB 77.83 kB
facebook-www/ReactDOMForked-prod.classic.js +0.45% 432.26 kB 434.19 kB +0.37% 79.28 kB 79.57 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show
Name +/- Base Current +/- gzip Base gzip Current gzip
oss-experimental/react-noop-renderer/cjs/react-noop-renderer.production.min.js +1.53% 13.82 kB 14.03 kB +1.07% 4.03 kB 4.07 kB
oss-stable-semver/react-noop-renderer/cjs/react-noop-renderer.production.min.js +1.53% 13.82 kB 14.03 kB +1.07% 4.03 kB 4.07 kB
oss-stable/react-noop-renderer/cjs/react-noop-renderer.production.min.js +1.53% 13.82 kB 14.03 kB +1.07% 4.03 kB 4.07 kB
oss-experimental/react-noop-renderer/cjs/react-noop-renderer-persistent.production.min.js +1.52% 13.88 kB 14.10 kB +1.09% 4.04 kB 4.09 kB
oss-stable-semver/react-noop-renderer/cjs/react-noop-renderer-persistent.production.min.js +1.52% 13.88 kB 14.10 kB +1.09% 4.04 kB 4.09 kB
oss-stable/react-noop-renderer/cjs/react-noop-renderer-persistent.production.min.js +1.52% 13.88 kB 14.10 kB +1.09% 4.04 kB 4.09 kB
oss-experimental/react-noop-renderer/cjs/react-noop-renderer.development.js +0.93% 42.26 kB 42.66 kB +0.89% 9.49 kB 9.57 kB
oss-stable-semver/react-noop-renderer/cjs/react-noop-renderer.development.js +0.93% 42.26 kB 42.66 kB +0.89% 9.49 kB 9.57 kB
oss-stable/react-noop-renderer/cjs/react-noop-renderer.development.js +0.93% 42.26 kB 42.66 kB +0.89% 9.49 kB 9.57 kB
oss-experimental/react-noop-renderer/cjs/react-noop-renderer-persistent.development.js +0.93% 42.40 kB 42.79 kB +0.87% 9.51 kB 9.59 kB
oss-stable-semver/react-noop-renderer/cjs/react-noop-renderer-persistent.development.js +0.93% 42.40 kB 42.79 kB +0.87% 9.51 kB 9.59 kB
oss-stable/react-noop-renderer/cjs/react-noop-renderer-persistent.development.js +0.93% 42.40 kB 42.79 kB +0.87% 9.51 kB 9.59 kB
facebook-www/ReactDOMForked-prod.modern.js +0.46% 422.15 kB 424.08 kB +0.36% 77.83 kB 78.11 kB
facebook-www/ReactDOMForked-prod.classic.js +0.45% 432.26 kB 434.19 kB +0.37% 79.28 kB 79.57 kB
facebook-www/ReactDOMForked-profiling.modern.js +0.42% 452.56 kB 454.47 kB +0.36% 82.37 kB 82.66 kB
facebook-www/ReactDOMForked-profiling.classic.js +0.41% 462.74 kB 464.64 kB +0.33% 83.89 kB 84.16 kB
oss-experimental/react/cjs/react-unstable-shared-subset.production.min.js +0.30% 6.59 kB 6.61 kB +0.30% 2.68 kB 2.69 kB
oss-stable-semver/react/cjs/react.production.min.js +0.28% 7.21 kB 7.23 kB +0.25% 2.84 kB 2.84 kB
oss-stable/react/cjs/react.production.min.js +0.28% 7.21 kB 7.23 kB +0.25% 2.84 kB 2.84 kB
oss-experimental/react/cjs/react.production.min.js +0.25% 7.88 kB 7.90 kB +0.20% 3.00 kB 3.01 kB
facebook-www/ReactDOMForked-dev.modern.js +0.23% 1,109.47 kB 1,112.04 kB +0.14% 245.63 kB 245.98 kB
facebook-www/ReactDOMForked-dev.classic.js +0.23% 1,131.79 kB 1,134.36 kB +0.13% 249.82 kB 250.15 kB

Generated by 🚫 dangerJS against 9ad9220

Copy link
Contributor

@bvaughn bvaughn left a comment

Choose a reason for hiding this comment

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

I think we'll also need to update DevTools in a few places to be aware of Tracing Markers, like

export function getDisplayNameForReactElement(

// NOTICE Keep in sync with shouldFilterFiber() and other get*ForFiber methods
function getDisplayNameForFiber(fiber: Fiber): string | null {
const {elementType, type, tag} = fiber;

import {
CONCURRENT_MODE_NUMBER,
CONCURRENT_MODE_SYMBOL_STRING,
DEPRECATED_ASYNC_MODE_SYMBOL_STRING,
PROVIDER_NUMBER,
PROVIDER_SYMBOL_STRING,
CONTEXT_NUMBER,
CONTEXT_SYMBOL_STRING,
STRICT_MODE_NUMBER,
STRICT_MODE_SYMBOL_STRING,
PROFILER_NUMBER,
PROFILER_SYMBOL_STRING,
SCOPE_NUMBER,
SCOPE_SYMBOL_STRING,
FORWARD_REF_NUMBER,
FORWARD_REF_SYMBOL_STRING,
MEMO_NUMBER,
MEMO_SYMBOL_STRING,
} from './ReactSymbols';

packages/react-dom/src/client/ReactDOMHostConfig.js Outdated Show resolved Hide resolved
packages/react-dom/src/client/ReactDOMHostConfig.js Outdated Show resolved Hide resolved
this.transitionCallbacks = null;
const transitionLanesMap = (this.transitionLanes = []);
for (let i = 0; i < TotalLanes; i++) {
transitionLanesMap.push(new Set());
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this being used anywhere yet?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope!

packages/react-dom/src/client/ReactDOMRoot.js Outdated Show resolved Hide resolved
@@ -243,6 +244,7 @@ export function getInternalReactConstants(
SimpleMemoComponent: 15,
SuspenseComponent: 13,
SuspenseListComponent: 19, // Experimental
TracingMarkerComponent: 25, // Experimental
Copy link
Contributor

Choose a reason for hiding this comment

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

Unfortunately I think this is wrong. We should probably add a new version check above for the 18 RC (whichever one first introduces the tracing marker), but so long as this is additive only– we could avoid forking the whole block and just rely on the act that we should never see a type-of-work value of 25 for React 17. What do you think?

Comment on lines 32 to 34
prevTransitionInfo = ReactCurrentBatchConfig.transitionInfo;
if (options !== undefined && options.name !== undefined) {
ReactCurrentBatchConfig.transitionInfo = {
name: options.name,
startTime: -1,
};
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this might be a logic bug?

If there's no new name, shouldn't we also (temporarily) clear out the previous transitionInfo before invoking the scope? or am I misreading this?

- add getCurrentEventStartTime (gets the start time of the current event if there is one and otherwise use performance.now
- add scheduleTransitionCallbacks to host configs, which lets you to schedule a list of transition callbacks that will be called sometime after paint. The DOM implementation is still being discussed
Add a transitionName to start transition, store the transition start time and name in the batch config, and pass it to the root on render
- Add transition pool stack to keep track of, for the current fiber, which transitions are active. This stack will be used like this:
       * On the root, we will get all transitions that have been initiated for the corresponding lanes.
       * Whenever we encounter a suspended boundary, we will add all transitions on the stack onto the boundary
       * Whenever we encounter a boundary that just unsuspended, we will add all transitions on the boundary onto the stack
   A transition will be considered complete when there are no boundaries that have the associated transition
- Add a tracing markers pool to keep track of, for the current fiber, what the parent tracing markers (and root) are. Tracing markers in the pool are represented by a TracingMarkerInfo object, which is:
       * The set of transitions that this tracing marker is keeping track of
       * A map of of pending boundaries that are in the tracing marker's subtree
- Add methods to get the transition for the current lanes (on the root transition lanes that we modified un the batch config commit) and to clear the transition lanes
- Adds the name field to Suspense and offscreen props. This field will be used in pending suspense boundaries to distinguish between which boundary is which.
- Because not all Suspense boundaries have names, we also generate a unique ID for each suspense boundary. We do this because, as mentioned earlier, the pending suspense boundaries map on tracing markers/roots contain only an object with a name that represent the suspense boundary and not the boundary itself. Therefore, this unique ID is a way to distinguish between different suspense boundaries.
- Store both the name and the unique ID on the Suspense Offscreen props so that they are preserved even after the Suspense boundary resolves.
The way we will fill the pending suspense boundaries map is as follows:
 - as we traverse down the tree, if we find any suspense offscreen components that first mounted or toggled between suspended/resolved, we will add a list of all tracing markers' pendingSuspenseBoundaries map to the update queue of the suspense boundary.
- during the commit phase, when we get to the suspense offscreen component, we will add/remove the corresponding boundary from all maps in the tracing marker pendingSuspenseBoundaries map. We will do this in the commit complete phase so that when we get back up to the parent tracing marker, the pending suspense boundaries will be up to date.

Whenever we update the tracing marker, we:
- Create a new transitions set, get all transitions currently in the transition pool, and all current transitions on tracing marker (only if the name hasn't changed) and add them to the transition set.
- Create a new pendingSuspenseBoundaries map if there wasn't one previously and use the old map if there was one. This map will eventually contain all pending suspense boundaries under the subtree, but for now it's empty.
- Add both the transitions set and the pending suspense boundaries map to the memoized state of the tracing marker.
- Push the pendingSuspenseBoundaries map onto the tracing markers pool stack
- if the suspense boundary went from fallback to a resolved state, push all transitions from the suspense boundary to the transition pool stack to propagate down
-  If the suspense boundary mounted in a fallback state, get all transitions on the transition pool stack and add them to the Offscreen memoized state
- If the suspense boundary updates and is in a fallback state, get all transitions on the transition pool stack AND all transitions on the boundary and store everything on the Offscreen memoized state
- Store the list of parent tracing marker info on the Offscreen update queue
The root operates as a tracing marker that has all transitions on it. This PR only tested the root with one transition so far

- Push root info onto the tracing marker pool stack
- The transitions on the root different from the tracing marker because once a tracing marker mounted, it can't have more transitions added to it unless it changes its name, in which case all transitions on it are deleted and the new transitions are added. However, the root is always accumulating more transitions. Therefore, we need to all all currently active transitions onto the root's memoizedState
- Create a pendingSuspenseBoundaries map that contains all pending suspense boundaries and add that to the root's memoizedState
- Do this in both updateHostRoot AND attemptEarlyBailoutIfNoScheduledUpdate. We need to do this in the latter part because even if the root itself doesn't have an update, it could still have new transitions in its transitionLanes map that we need to process.
Because we pushed things onto the transition and tracing marker stacks in the begin phase, we now need to pop everything off the stack as well in the complete phase to ensure that the stacks are correct
- Adds the SuspenseToggle flag and subtree flag, which we set if the suspense component mounted in a fallback state OR if a component toggled between fallback/resolved states. This is because if this is the case, we'll need to update the pendingSuspenseBoundaries map in the commit phase
- Add the SuspenseToggle flag if the subtree flag is set. This means that a child has toggled the SuspenseToggle flag, so the TracingMarker might have to call a callback
- adds a module scoped pending transition callbacks object that contains all transition callbacks that have not yet been processed. This  contains all callbacks before the next paint occurs. 
- Add code in the mutation phase to:
        * For the root, if there are transitions that were initialized during this commit in the root transition lanes map, add a transition start call to the pending transition callbacks object. Then, remove the transitions from the root transition lanes map.
        * For tracing markers and roots, if the pending suspense boundary map has no boundaries, this means that the transition must be complete. Add a transition complete call
        * For tracing markers and roots, always add a transition progress call
         * For offscreen components, add or remove boundary info on the pending boundaries map. We will add the boundary info if the suspense boundary toggles to a fallback or if it initially mounts in a fallback state. We will remove the boundary info if the boundary toggles from a fallback to a resolved state

We add this code in the mutation phase because we can't add it to the passive phase because then the paint might have occurred before we even know which callbacks to call
At the end of the commit phase, call scheduleTransitionCallbacks to schedule all pending transition callbacks to be called after paint. Then clear the callbacks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed React Core Team Opened by a member of the React Core Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants