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.

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

Jump to bottom

[Transition Tracing] Add transition tracing transitions stack #24321

Merged
merged 1 commit into from
Apr 9, 2022
+162 −88
Merged

[Transition Tracing] Add transition tracing transitions stack #24321

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
17 changes: 10 additions & 7 deletions packages/react-reconciler/src/ReactFiberBeginWork.new.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -661,7 +661,7 @@ function updateOffscreenComponent(
// push the cache pool even though we're going to bail out
// because otherwise there'd be a context mismatch
if (current !== null) {
pushTransition(workInProgress, null);
pushTransition(workInProgress, null, null);
}
}
pushRenderLanes(workInProgress, renderLanes);
Expand Down Expand Up @@ -695,7 +695,7 @@ function updateOffscreenComponent(
// push the cache pool even though we're going to bail out
// because otherwise there'd be a context mismatch
if (current !== null) {
pushTransition(workInProgress, null);
pushTransition(workInProgress, null, null);
}
}

Expand Down Expand Up @@ -733,7 +733,9 @@ function updateOffscreenComponent(
// using the same cache. Unless the parent changed, since that means
// there was a refresh.
const prevCachePool = prevState !== null ? prevState.cachePool : null;
pushTransition(workInProgress, prevCachePool);
// TODO: Consider if and how Offscreen pre-rendering should
// be attributed to the transition that spawned it
pushTransition(workInProgress, prevCachePool, null);
}

pushRenderLanes(workInProgress, subtreeRenderLanes);
Expand All @@ -751,7 +753,7 @@ function updateOffscreenComponent(
// using the same cache. Unless the parent changed, since that means
// there was a refresh.
const prevCachePool = prevState.cachePool;
pushTransition(workInProgress, prevCachePool);
pushTransition(workInProgress, prevCachePool, null);
}

// Since we're not hidden anymore, reset the state
Expand All @@ -767,7 +769,7 @@ function updateOffscreenComponent(
// using the same cache. Unless the parent changed, since that means
// there was a refresh.
if (current !== null) {
pushTransition(workInProgress, null);
pushTransition(workInProgress, null, null);
}
}
}
Expand Down Expand Up @@ -1330,10 +1332,10 @@ function updateHostRoot(current, workInProgress, renderLanes) {

const nextState: RootState = workInProgress.memoizedState;
const root: FiberRoot = workInProgress.stateNode;
pushRootTransition(workInProgress, root, renderLanes);

if (enableCache) {
const nextCache: Cache = nextState.cache;
pushRootTransition(root);
acdlite marked this conversation as resolved.
Show resolved Hide resolved
pushCacheProvider(workInProgress, nextCache);
if (nextCache !== prevState.cache) {
// The root cache refreshed.
Expand Down Expand Up @@ -3572,10 +3574,11 @@ function attemptEarlyBailoutIfNoScheduledUpdate(
case HostRoot:
pushHostRootContext(workInProgress);
const root: FiberRoot = workInProgress.stateNode;
pushRootTransition(workInProgress, root, renderLanes);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: the order of pushes in the begin phase needs to be the reverse of the pops in the complete phase.

It happens to not matter here because none of these functions read the current value before the next one pops, but it's a refactor hazard if we ever change them to do that.

So since the complete phase is:

  • popRootTransition
  • popCacheProvider

The begin phase needs to be:

  • pushCacheProvider
  • pushRootTransition


if (enableCache) {
const cache: Cache = current.memoizedState.cache;
pushCacheProvider(workInProgress, cache);
pushRootTransition(root);
acdlite marked this conversation as resolved.
Show resolved Hide resolved
}
if (enableTransitionTracing) {
workInProgress.memoizedState.transitions = getWorkInProgressTransitions();
Expand Down
17 changes: 10 additions & 7 deletions packages/react-reconciler/src/ReactFiberBeginWork.old.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -661,7 +661,7 @@ function updateOffscreenComponent(
// push the cache pool even though we're going to bail out
// because otherwise there'd be a context mismatch
if (current !== null) {
pushTransition(workInProgress, null);
pushTransition(workInProgress, null, null);
}
}
pushRenderLanes(workInProgress, renderLanes);
Expand Down Expand Up @@ -695,7 +695,7 @@ function updateOffscreenComponent(
// push the cache pool even though we're going to bail out
// because otherwise there'd be a context mismatch
if (current !== null) {
pushTransition(workInProgress, null);
pushTransition(workInProgress, null, null);
}
}

Expand Down Expand Up @@ -733,7 +733,9 @@ function updateOffscreenComponent(
// using the same cache. Unless the parent changed, since that means
// there was a refresh.
const prevCachePool = prevState !== null ? prevState.cachePool : null;
pushTransition(workInProgress, prevCachePool);
// TODO: Consider if and how Offscreen pre-rendering should
// be attributed to the transition that spawned it
pushTransition(workInProgress, prevCachePool, null);
}

pushRenderLanes(workInProgress, subtreeRenderLanes);
Expand All @@ -751,7 +753,7 @@ function updateOffscreenComponent(
// using the same cache. Unless the parent changed, since that means
// there was a refresh.
const prevCachePool = prevState.cachePool;
pushTransition(workInProgress, prevCachePool);
pushTransition(workInProgress, prevCachePool, null);
}

// Since we're not hidden anymore, reset the state
Expand All @@ -767,7 +769,7 @@ function updateOffscreenComponent(
// using the same cache. Unless the parent changed, since that means
// there was a refresh.
if (current !== null) {
pushTransition(workInProgress, null);
pushTransition(workInProgress, null, null);
}
}
}
Expand Down Expand Up @@ -1330,10 +1332,10 @@ function updateHostRoot(current, workInProgress, renderLanes) {

const nextState: RootState = workInProgress.memoizedState;
const root: FiberRoot = workInProgress.stateNode;
pushRootTransition(workInProgress, root, renderLanes);

if (enableCache) {
const nextCache: Cache = nextState.cache;
pushRootTransition(root);
pushCacheProvider(workInProgress, nextCache);
if (nextCache !== prevState.cache) {
// The root cache refreshed.
Expand Down Expand Up @@ -3572,10 +3574,11 @@ function attemptEarlyBailoutIfNoScheduledUpdate(
case HostRoot:
pushHostRootContext(workInProgress);
const root: FiberRoot = workInProgress.stateNode;
pushRootTransition(workInProgress, root, renderLanes);

if (enableCache) {
const cache: Cache = current.memoizedState.cache;
pushCacheProvider(workInProgress, cache);
pushRootTransition(root);
}
if (enableTransitionTracing) {
workInProgress.memoizedState.transitions = getWorkInProgressTransitions();
Expand Down
8 changes: 3 additions & 5 deletions packages/react-reconciler/src/ReactFiberCompleteWork.new.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -875,8 +875,6 @@ function completeWork(
}

if (enableCache) {
popRootTransition(fiberRoot, renderLanes);

let previousCache: Cache | null = null;
if (current !== null) {
previousCache = current.memoizedState.cache;
Expand All @@ -888,6 +886,7 @@ function completeWork(
}
popCacheProvider(workInProgress, cache);
}
popRootTransition(workInProgress, fiberRoot, renderLanes);
popHostContainer(workInProgress);
popTopLevelLegacyContextObject(workInProgress);
resetMutableSourceWorkInProgressVersions();
Expand Down Expand Up @@ -1593,11 +1592,10 @@ function completeWork(
// Run passive effects to retain/release the cache.
workInProgress.flags |= Passive;
}
if (current !== null) {
popTransition(workInProgress);
}
}

popTransition(workInProgress, current);
acdlite marked this conversation as resolved.
Show resolved Hide resolved

return null;
}
case CacheComponent: {
Expand Down
8 changes: 3 additions & 5 deletions packages/react-reconciler/src/ReactFiberCompleteWork.old.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -875,8 +875,6 @@ function completeWork(
}

if (enableCache) {
popRootTransition(fiberRoot, renderLanes);

let previousCache: Cache | null = null;
if (current !== null) {
previousCache = current.memoizedState.cache;
Expand All @@ -888,6 +886,7 @@ function completeWork(
}
popCacheProvider(workInProgress, cache);
}
popRootTransition(workInProgress, fiberRoot, renderLanes);
popHostContainer(workInProgress);
popTopLevelLegacyContextObject(workInProgress);
resetMutableSourceWorkInProgressVersions();
Expand Down Expand Up @@ -1593,11 +1592,10 @@ function completeWork(
// Run passive effects to retain/release the cache.
workInProgress.flags |= Passive;
}
if (current !== null) {
popTransition(workInProgress);
}
}

popTransition(workInProgress, current);

return null;
}
case CacheComponent: {
Expand Down
77 changes: 62 additions & 15 deletions packages/react-reconciler/src/ReactFiberTransition.new.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,15 @@ import type {FiberRoot} from './ReactInternalTypes';
import type {Lanes} from './ReactFiberLane.new';
import type {StackCursor} from './ReactFiberStack.new';
import type {Cache, SpawnedCachePool} from './ReactFiberCacheComponent.new';
import type {Transition} from './ReactFiberTracingMarkerComponent.new';

import {enableCache} from 'shared/ReactFeatureFlags';
import {enableCache, enableTransitionTracing} from 'shared/ReactFeatureFlags';
import {isPrimaryRenderer} from './ReactFiberHostConfig';
import {createCursor, push, pop} from './ReactFiberStack.new';
import {getWorkInProgressRoot} from './ReactFiberWorkLoop.new';
import {
getWorkInProgressRoot,
getWorkInProgressTransitions,
} from './ReactFiberWorkLoop.new';
import {
createCache,
retainCache,
Expand All @@ -25,6 +29,15 @@ import {
// used during the previous render by placing it here, on the stack.
const resumedCache: StackCursor<Cache | null> = createCursor(null);

// During the render/synchronous commit phase, we don't actually process the
// transitions. Therefore, we want to lazily combine transitions. Instead of
// comparing the arrays of transitions when we combine them and storing them
// and filtering out the duplicates, we will instead store the unprocessed transitions
// in an array and actually filter them in the passive phase.
const transitionStack: StackCursor<Array<Transition> | null> = createCursor(
null,
);

function peekCacheFromPool(): Cache | null {
if (!enableCache) {
return (null: any);
Expand Down Expand Up @@ -75,25 +88,31 @@ export function requestCacheFromPool(renderLanes: Lanes): Cache {
return freshCache;
}

export function pushRootTransition(root: FiberRoot) {
if (enableCache) {
return;
export function pushRootTransition(
workInProgress: Fiber,
root: FiberRoot,
renderLanes: Lanes,
) {
if (enableTransitionTracing) {
const rootTransitions = getWorkInProgressTransitions();
push(transitionStack, rootTransitions, workInProgress);
}
// Note: This function currently does nothing but I'll leave it here for
// code organization purposes in case that changes.
}

export function popRootTransition(root: FiberRoot, renderLanes: Lanes) {
if (enableCache) {
return;
export function popRootTransition(
workInProgress: Fiber,
root: FiberRoot,
renderLanes: Lanes,
) {
if (enableTransitionTracing) {
pop(transitionStack, workInProgress);
}
// Note: This function currently does nothing but I'll leave it here for
// code organization purposes in case that changes.
}

export function pushTransition(
offscreenWorkInProgress: Fiber,
prevCachePool: SpawnedCachePool | null,
newTransitions: Array<Transition> | null,
): void {
if (enableCache) {
if (prevCachePool === null) {
Expand All @@ -102,12 +121,40 @@ export function pushTransition(
push(resumedCache, prevCachePool.pool, offscreenWorkInProgress);
}
}

if (enableTransitionTracing) {
if (transitionStack.current === null) {
push(transitionStack, newTransitions, offscreenWorkInProgress);
} else if (newTransitions === null) {
push(transitionStack, transitionStack.current, offscreenWorkInProgress);
} else {
push(
transitionStack,
transitionStack.current.concat(newTransitions),
offscreenWorkInProgress,
);
}
}
}

export function popTransition(workInProgress: Fiber) {
if (enableCache) {
pop(resumedCache, workInProgress);
export function popTransition(workInProgress: Fiber, current: Fiber | null) {
if (current !== null) {
if (enableCache) {
pop(resumedCache, workInProgress);
}

if (enableTransitionTracing) {
pop(transitionStack, workInProgress);
}
}
}

export function getSuspendedTransitions(): Array<Transition> | null {
if (!enableTransitionTracing) {
return null;
}

return transitionStack.current;
}

export function getSuspendedCache(): SpawnedCachePool | null {
Expand Down
Loading