From a30d15f8df39b9140283a88cdae58ac02435826a Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Tue, 26 Jan 2021 23:19:25 -0600 Subject: [PATCH] Entangle overlapping transitions per queue When multiple transitions update the same queue, only the most recent one should be allowed to finish. We shouldn't show intermediate states. See #17418 for background on why this is important. The way this currently works is that we always assign the same lane to all transitions. It's impossible for one transition to finish without also finishing all the others. The downside of the current approach is that it's too aggressive. Not all transitions are related to each other, so one should not block the other. The new approach is to only entangle transitions if they update one or more of the same state hooks (or class components), because this indicates that they are related. If they are unrelated, then they can finish in any order, as long as they have different lanes. However, this commit does not change anything about how the lanes are assigned. All it does is add the mechanism to entangle per queue. So it doesn't actually change any behavior, yet. But it's a requirement for my next step, which is to assign different lanes to consecutive transitions until we run out and cycle back to the beginning. --- .../src/ReactFiberClassComponent.new.js | 16 ++++++-- .../src/ReactFiberClassComponent.old.js | 16 ++++++-- .../src/ReactFiberHooks.new.js | 39 +++++++++++++++++- .../src/ReactFiberHooks.old.js | 41 ++++++++++++++++++- .../src/ReactFiberLane.new.js | 13 ++++-- .../src/ReactFiberLane.old.js | 13 ++++-- .../src/ReactFiberReconciler.new.js | 11 ++++- .../src/ReactFiberReconciler.old.js | 11 ++++- .../src/ReactUpdateQueue.new.js | 38 ++++++++++++++++- .../src/ReactUpdateQueue.old.js | 40 +++++++++++++++++- 10 files changed, 216 insertions(+), 22 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberClassComponent.new.js b/packages/react-reconciler/src/ReactFiberClassComponent.new.js index 9ed6dc23c9f16..389b4877852db 100644 --- a/packages/react-reconciler/src/ReactFiberClassComponent.new.js +++ b/packages/react-reconciler/src/ReactFiberClassComponent.new.js @@ -40,6 +40,7 @@ import { import { enqueueUpdate, + entangleTransitions, processUpdateQueue, checkHasForceUpdateAfterProcessing, resetHasForceUpdateBeforeProcessing, @@ -214,7 +215,10 @@ const classComponentUpdater = { } enqueueUpdate(fiber, update, lane); - scheduleUpdateOnFiber(fiber, lane, eventTime); + const root = scheduleUpdateOnFiber(fiber, lane, eventTime); + if (root !== null) { + entangleTransitions(root, fiber, lane); + } if (__DEV__) { if (enableDebugTracing) { @@ -246,7 +250,10 @@ const classComponentUpdater = { } enqueueUpdate(fiber, update, lane); - scheduleUpdateOnFiber(fiber, lane, eventTime); + const root = scheduleUpdateOnFiber(fiber, lane, eventTime); + if (root !== null) { + entangleTransitions(root, fiber, lane); + } if (__DEV__) { if (enableDebugTracing) { @@ -277,7 +284,10 @@ const classComponentUpdater = { } enqueueUpdate(fiber, update, lane); - scheduleUpdateOnFiber(fiber, lane, eventTime); + const root = scheduleUpdateOnFiber(fiber, lane, eventTime); + if (root !== null) { + entangleTransitions(root, fiber, lane); + } if (__DEV__) { if (enableDebugTracing) { diff --git a/packages/react-reconciler/src/ReactFiberClassComponent.old.js b/packages/react-reconciler/src/ReactFiberClassComponent.old.js index 2b4496ca5c910..36bf2d22f75f6 100644 --- a/packages/react-reconciler/src/ReactFiberClassComponent.old.js +++ b/packages/react-reconciler/src/ReactFiberClassComponent.old.js @@ -40,6 +40,7 @@ import { import { enqueueUpdate, + entangleTransitions, processUpdateQueue, checkHasForceUpdateAfterProcessing, resetHasForceUpdateBeforeProcessing, @@ -214,7 +215,10 @@ const classComponentUpdater = { } enqueueUpdate(fiber, update); - scheduleUpdateOnFiber(fiber, lane, eventTime); + const root = scheduleUpdateOnFiber(fiber, lane, eventTime); + if (root !== null) { + entangleTransitions(root, fiber, lane); + } if (__DEV__) { if (enableDebugTracing) { @@ -246,7 +250,10 @@ const classComponentUpdater = { } enqueueUpdate(fiber, update); - scheduleUpdateOnFiber(fiber, lane, eventTime); + const root = scheduleUpdateOnFiber(fiber, lane, eventTime); + if (root !== null) { + entangleTransitions(root, fiber, lane); + } if (__DEV__) { if (enableDebugTracing) { @@ -277,7 +284,10 @@ const classComponentUpdater = { } enqueueUpdate(fiber, update); - scheduleUpdateOnFiber(fiber, lane, eventTime); + const root = scheduleUpdateOnFiber(fiber, lane, eventTime); + if (root !== null) { + entangleTransitions(root, fiber, lane); + } if (__DEV__) { if (enableDebugTracing) { diff --git a/packages/react-reconciler/src/ReactFiberHooks.new.js b/packages/react-reconciler/src/ReactFiberHooks.new.js index 374d383030cdb..b615ae1e5e221 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.new.js +++ b/packages/react-reconciler/src/ReactFiberHooks.new.js @@ -45,6 +45,8 @@ import { isSubsetOfLanes, mergeLanes, removeLanes, + intersectLanes, + isTransitionLane, markRootEntangled, markRootMutableRead, getCurrentUpdateLanePriority, @@ -104,7 +106,11 @@ import {getIsRendering} from './ReactCurrentFiber'; import {logStateUpdateScheduled} from './DebugTracing'; import {markStateUpdateScheduled} from './SchedulingProfiler'; import {CacheContext} from './ReactFiberCacheComponent.new'; -import {createUpdate, enqueueUpdate} from './ReactUpdateQueue.new'; +import { + createUpdate, + enqueueUpdate, + entangleTransitions, +} from './ReactUpdateQueue.new'; import {pushInterleavedQueue} from './ReactFiberInterleavedUpdates.new'; const {ReactCurrentDispatcher, ReactCurrentBatchConfig} = ReactSharedInternals; @@ -121,6 +127,7 @@ type Update = {| export type UpdateQueue = {| pending: Update | null, interleaved: Update | null, + lanes: Lanes, dispatch: (A => mixed) | null, lastRenderedReducer: ((S, A) => S) | null, lastRenderedState: S | null, @@ -654,6 +661,7 @@ function mountReducer( const queue = (hook.queue = { pending: null, interleaved: null, + lanes: NoLanes, dispatch: null, lastRenderedReducer: reducer, lastRenderedState: (initialState: any), @@ -811,6 +819,10 @@ function updateReducer( markSkippedUpdateLanes(interleavedLane); interleaved = ((interleaved: any).next: Update); } while (interleaved !== lastInterleaved); + } else if (baseQueue === null) { + // `queue.lanes` is used for entangling transitions. We can set it back to + // zero once the queue is empty. + queue.lanes = NoLanes; } const dispatch: Dispatch = (queue.dispatch: any); @@ -1102,6 +1114,7 @@ function useMutableSource( const newQueue = { pending: null, interleaved: null, + lanes: NoLanes, dispatch: null, lastRenderedReducer: basicStateReducer, lastRenderedState: snapshot, @@ -1158,6 +1171,7 @@ function mountState( const queue = (hook.queue = { pending: null, interleaved: null, + lanes: NoLanes, dispatch: null, lastRenderedReducer: basicStateReducer, lastRenderedState: (initialState: any), @@ -1821,6 +1835,9 @@ function refreshCache(fiber: Fiber, seedKey: ?() => T, seedValue: T) { const lane = requestUpdateLane(provider); const eventTime = requestEventTime(); const root = scheduleUpdateOnFiber(provider, lane, eventTime); + if (root !== null) { + entangleTransitions(root, fiber, lane); + } const seededCache = new Map(); if (seedKey !== null && seedKey !== undefined && root !== null) { @@ -1960,7 +1977,25 @@ function dispatchAction( warnIfNotCurrentlyActingUpdatesInDev(fiber); } } - scheduleUpdateOnFiber(fiber, lane, eventTime); + const root = scheduleUpdateOnFiber(fiber, lane, eventTime); + + if (isTransitionLane(lane) && root !== null) { + let queueLanes = queue.lanes; + + // If any entangled lanes are no longer pending on the root, then they + // must have finished. We can remove them from the shared queue, which + // represents a superset of the actually pending lanes. In some cases we + // may entangle more than we need to, but that's OK. In fact it's worse if + // we *don't* entangle when we should. + queueLanes = intersectLanes(queueLanes, root.pendingLanes); + + // Entangle the new transition lane with the other transition lanes. + const newQueueLanes = mergeLanes(queueLanes, lane); + if (newQueueLanes !== queueLanes) { + queue.lanes = newQueueLanes; + markRootEntangled(root, newQueueLanes); + } + } } if (__DEV__) { diff --git a/packages/react-reconciler/src/ReactFiberHooks.old.js b/packages/react-reconciler/src/ReactFiberHooks.old.js index 26721dcd11264..dfde5f7aa116d 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.old.js +++ b/packages/react-reconciler/src/ReactFiberHooks.old.js @@ -45,6 +45,8 @@ import { isSubsetOfLanes, mergeLanes, removeLanes, + intersectLanes, + isTransitionLane, markRootEntangled, markRootMutableRead, getCurrentUpdateLanePriority, @@ -103,7 +105,11 @@ import {getIsRendering} from './ReactCurrentFiber'; import {logStateUpdateScheduled} from './DebugTracing'; import {markStateUpdateScheduled} from './SchedulingProfiler'; import {CacheContext} from './ReactFiberCacheComponent.old'; -import {createUpdate, enqueueUpdate} from './ReactUpdateQueue.old'; +import { + createUpdate, + enqueueUpdate, + entangleTransitions, +} from './ReactUpdateQueue.old'; const {ReactCurrentDispatcher, ReactCurrentBatchConfig} = ReactSharedInternals; @@ -118,6 +124,7 @@ type Update = {| type UpdateQueue = {| pending: Update | null, + lanes: Lanes, dispatch: (A => mixed) | null, lastRenderedReducer: ((S, A) => S) | null, lastRenderedState: S | null, @@ -650,6 +657,7 @@ function mountReducer( hook.memoizedState = hook.baseState = initialState; const queue = (hook.queue = { pending: null, + lanes: NoLanes, dispatch: null, lastRenderedReducer: reducer, lastRenderedState: (initialState: any), @@ -792,6 +800,12 @@ function updateReducer( queue.lastRenderedState = newState; } + if (baseQueue === null) { + // `queue.lanes` is used for entangling transitions. We can set it back to + // zero once the queue is empty. + queue.lanes = NoLanes; + } + const dispatch: Dispatch = (queue.dispatch: any); return [hook.memoizedState, dispatch]; } @@ -1080,6 +1094,7 @@ function useMutableSource( // including any interleaving updates that occur. const newQueue = { pending: null, + lanes: NoLanes, dispatch: null, lastRenderedReducer: basicStateReducer, lastRenderedState: snapshot, @@ -1135,6 +1150,7 @@ function mountState( hook.memoizedState = hook.baseState = initialState; const queue = (hook.queue = { pending: null, + lanes: NoLanes, dispatch: null, lastRenderedReducer: basicStateReducer, lastRenderedState: (initialState: any), @@ -1798,6 +1814,9 @@ function refreshCache(fiber: Fiber, seedKey: ?() => T, seedValue: T) { const lane = requestUpdateLane(provider); const eventTime = requestEventTime(); const root = scheduleUpdateOnFiber(provider, lane, eventTime); + if (root !== null) { + entangleTransitions(root, fiber, lane); + } const seededCache = new Map(); if (seedKey !== null && seedKey !== undefined && root !== null) { @@ -1914,7 +1933,25 @@ function dispatchAction( warnIfNotCurrentlyActingUpdatesInDev(fiber); } } - scheduleUpdateOnFiber(fiber, lane, eventTime); + const root = scheduleUpdateOnFiber(fiber, lane, eventTime); + + if (isTransitionLane(lane) && root !== null) { + let queueLanes = queue.lanes; + + // If any entangled lanes are no longer pending on the root, then they + // must have finished. We can remove them from the shared queue, which + // represents a superset of the actually pending lanes. In some cases we + // may entangle more than we need to, but that's OK. In fact it's worse if + // we *don't* entangle when we should. + queueLanes = intersectLanes(queueLanes, root.pendingLanes); + + // Entangle the new transition lane with the other transition lanes. + const newQueueLanes = mergeLanes(queueLanes, lane); + if (newQueueLanes !== queueLanes) { + queue.lanes = newQueueLanes; + markRootEntangled(root, newQueueLanes); + } + } } if (__DEV__) { diff --git a/packages/react-reconciler/src/ReactFiberLane.new.js b/packages/react-reconciler/src/ReactFiberLane.new.js index 4570c10a62b42..88c4d1946cb6d 100644 --- a/packages/react-reconciler/src/ReactFiberLane.new.js +++ b/packages/react-reconciler/src/ReactFiberLane.new.js @@ -310,9 +310,8 @@ export function getNextLanes(root: FiberRoot, wipLanes: Lanes): Lanes { } if (enableTransitionEntanglement) { - // We don't need to include higher priority lanes, because in this - // experiment we always unsuspend all transitions whenever we receive - // an update. + // We don't need to do anything extra here, because we apply per-lane + // transition entanglement in the entanglement loop below. } else { // If there are higher priority lanes, we'll include them even if they // are suspended. @@ -492,6 +491,10 @@ export function includesOnlyTransitions(lanes: Lanes) { return (lanes & TransitionLanes) === lanes; } +export function isTransitionLane(lane: Lane) { + return (lane & TransitionLanes) !== 0; +} + // To ensure consistency across multiple updates in the same event, this should // be a pure function, so that it always returns the same lane for given inputs. export function findUpdateLane( @@ -634,6 +637,10 @@ export function removeLanes(set: Lanes, subset: Lanes | Lane): Lanes { return set & ~subset; } +export function intersectLanes(a: Lanes | Lane, b: Lanes | Lane): Lanes { + return a & b; +} + // Seems redundant, but it changes the type from a single lane (used for // updates) to a group of lanes (used for flushing work). export function laneToLanes(lane: Lane): Lanes { diff --git a/packages/react-reconciler/src/ReactFiberLane.old.js b/packages/react-reconciler/src/ReactFiberLane.old.js index 7be355d2c2458..a083ae68d8a8e 100644 --- a/packages/react-reconciler/src/ReactFiberLane.old.js +++ b/packages/react-reconciler/src/ReactFiberLane.old.js @@ -310,9 +310,8 @@ export function getNextLanes(root: FiberRoot, wipLanes: Lanes): Lanes { } if (enableTransitionEntanglement) { - // We don't need to include higher priority lanes, because in this - // experiment we always unsuspend all transitions whenever we receive - // an update. + // We don't need to do anything extra here, because we apply per-lane + // transition entanglement in the entanglement loop below. } else { // If there are higher priority lanes, we'll include them even if they // are suspended. @@ -492,6 +491,10 @@ export function includesOnlyTransitions(lanes: Lanes) { return (lanes & TransitionLanes) === lanes; } +export function isTransitionLane(lane: Lane) { + return (lane & TransitionLanes) !== 0; +} + // To ensure consistency across multiple updates in the same event, this should // be a pure function, so that it always returns the same lane for given inputs. export function findUpdateLane( @@ -634,6 +637,10 @@ export function removeLanes(set: Lanes, subset: Lanes | Lane): Lanes { return set & ~subset; } +export function intersectLanes(a: Lanes | Lane, b: Lanes | Lane): Lanes { + return a & b; +} + // Seems redundant, but it changes the type from a single lane (used for // updates) to a group of lanes (used for flushing work). export function laneToLanes(lane: Lane): Lanes { diff --git a/packages/react-reconciler/src/ReactFiberReconciler.new.js b/packages/react-reconciler/src/ReactFiberReconciler.new.js index 08ec3c8eac87e..70923424e8870 100644 --- a/packages/react-reconciler/src/ReactFiberReconciler.new.js +++ b/packages/react-reconciler/src/ReactFiberReconciler.new.js @@ -65,7 +65,11 @@ import { IsThisRendererActing, act, } from './ReactFiberWorkLoop.new'; -import {createUpdate, enqueueUpdate} from './ReactUpdateQueue.new'; +import { + createUpdate, + enqueueUpdate, + entangleTransitions, +} from './ReactUpdateQueue.new'; import { isRendering as ReactCurrentFiberIsRendering, current as ReactCurrentFiberCurrent, @@ -315,7 +319,10 @@ export function updateContainer( } enqueueUpdate(current, update, lane); - scheduleUpdateOnFiber(current, lane, eventTime); + const root = scheduleUpdateOnFiber(current, lane, eventTime); + if (root !== null) { + entangleTransitions(root, current, lane); + } return lane; } diff --git a/packages/react-reconciler/src/ReactFiberReconciler.old.js b/packages/react-reconciler/src/ReactFiberReconciler.old.js index 0fc72c42a07e0..1fc12435d64cf 100644 --- a/packages/react-reconciler/src/ReactFiberReconciler.old.js +++ b/packages/react-reconciler/src/ReactFiberReconciler.old.js @@ -65,7 +65,11 @@ import { IsThisRendererActing, act, } from './ReactFiberWorkLoop.old'; -import {createUpdate, enqueueUpdate} from './ReactUpdateQueue.old'; +import { + createUpdate, + enqueueUpdate, + entangleTransitions, +} from './ReactUpdateQueue.old'; import { isRendering as ReactCurrentFiberIsRendering, current as ReactCurrentFiberCurrent, @@ -315,7 +319,10 @@ export function updateContainer( } enqueueUpdate(current, update); - scheduleUpdateOnFiber(current, lane, eventTime); + const root = scheduleUpdateOnFiber(current, lane, eventTime); + if (root !== null) { + entangleTransitions(root, current, lane); + } return lane; } diff --git a/packages/react-reconciler/src/ReactUpdateQueue.new.js b/packages/react-reconciler/src/ReactUpdateQueue.new.js index 84575950c3465..205d7d21d501d 100644 --- a/packages/react-reconciler/src/ReactUpdateQueue.new.js +++ b/packages/react-reconciler/src/ReactUpdateQueue.new.js @@ -84,7 +84,7 @@ // regardless of priority. Intermediate state may vary according to system // resources, but the final state is always the same. -import type {Fiber} from './ReactInternalTypes'; +import type {Fiber, FiberRoot} from './ReactInternalTypes'; import type {Lanes, Lane} from './ReactFiberLane.new'; import { @@ -92,6 +92,9 @@ import { NoLanes, isSubsetOfLanes, mergeLanes, + isTransitionLane, + intersectLanes, + markRootEntangled, } from './ReactFiberLane.new'; import { enterDisallowedContextReadInDEV, @@ -128,6 +131,7 @@ export type Update = {| export type SharedQueue = {| pending: Update | null, interleaved: Update | null, + lanes: Lanes, |}; export type UpdateQueue = {| @@ -167,6 +171,7 @@ export function initializeUpdateQueue(fiber: Fiber): void { shared: { pending: null, interleaved: null, + lanes: NoLanes, }, effects: null, }; @@ -260,6 +265,33 @@ export function enqueueUpdate( } } +export function entangleTransitions(root: FiberRoot, fiber: Fiber, lane: Lane) { + const updateQueue = fiber.updateQueue; + if (updateQueue === null) { + // Only occurs if the fiber has been unmounted. + return; + } + + const sharedQueue: SharedQueue = (updateQueue: any).shared; + if (isTransitionLane(lane)) { + let queueLanes = sharedQueue.lanes; + + // If any entangled lanes are no longer pending on the root, then they must + // have finished. We can remove them from the shared queue, which represents + // a superset of the actually pending lanes. In some cases we may entangle + // more than we need to, but that's OK. In fact it's worse if we *don't* + // entangle when we should. + queueLanes = intersectLanes(queueLanes, root.pendingLanes); + + // Entangle the new transition lane with the other transition lanes. + const newQueueLanes = mergeLanes(queueLanes, lane); + if (newQueueLanes !== queueLanes) { + sharedQueue.lanes = newQueueLanes; + markRootEntangled(root, newQueueLanes); + } + } +} + export function enqueueCapturedUpdate( workInProgress: Fiber, capturedUpdate: Update, @@ -595,6 +627,10 @@ export function processUpdateQueue( newLanes = mergeLanes(newLanes, interleaved.lane); interleaved = ((interleaved: any).next: Update); } while (interleaved !== lastInterleaved); + } else if (firstBaseUpdate === null) { + // `queue.lanes` is used for entangling transitions. We can set it back to + // zero once the queue is empty. + queue.shared.lanes = NoLanes; } // Set the remaining expiration time to be whatever is remaining in the queue. diff --git a/packages/react-reconciler/src/ReactUpdateQueue.old.js b/packages/react-reconciler/src/ReactUpdateQueue.old.js index f1c14537a42d3..1d71e14819d87 100644 --- a/packages/react-reconciler/src/ReactUpdateQueue.old.js +++ b/packages/react-reconciler/src/ReactUpdateQueue.old.js @@ -84,7 +84,7 @@ // regardless of priority. Intermediate state may vary according to system // resources, but the final state is always the same. -import type {Fiber} from './ReactInternalTypes'; +import type {Fiber, FiberRoot} from './ReactInternalTypes'; import type {Lanes, Lane} from './ReactFiberLane.old'; import { @@ -92,6 +92,9 @@ import { NoLanes, isSubsetOfLanes, mergeLanes, + isTransitionLane, + intersectLanes, + markRootEntangled, } from './ReactFiberLane.old'; import { enterDisallowedContextReadInDEV, @@ -123,6 +126,7 @@ export type Update = {| type SharedQueue = {| pending: Update | null, + lanes: Lanes, |}; export type UpdateQueue = {| @@ -161,6 +165,7 @@ export function initializeUpdateQueue(fiber: Fiber): void { lastBaseUpdate: null, shared: { pending: null, + lanes: NoLanes, }, effects: null, }; @@ -234,6 +239,33 @@ export function enqueueUpdate(fiber: Fiber, update: Update) { } } +export function entangleTransitions(root: FiberRoot, fiber: Fiber, lane: Lane) { + const updateQueue = fiber.updateQueue; + if (updateQueue === null) { + // Only occurs if the fiber has been unmounted. + return; + } + + const sharedQueue: SharedQueue = (updateQueue: any).shared; + if (isTransitionLane(lane)) { + let queueLanes = sharedQueue.lanes; + + // If any entangled lanes are no longer pending on the root, then they must + // have finished. We can remove them from the shared queue, which represents + // a superset of the actually pending lanes. In some cases we may entangle + // more than we need to, but that's OK. In fact it's worse if we *don't* + // entangle when we should. + queueLanes = intersectLanes(queueLanes, root.pendingLanes); + + // Entangle the new transition lane with the other transition lanes. + const newQueueLanes = mergeLanes(queueLanes, lane); + if (newQueueLanes !== queueLanes) { + sharedQueue.lanes = newQueueLanes; + markRootEntangled(root, newQueueLanes); + } + } +} + export function enqueueCapturedUpdate( workInProgress: Fiber, capturedUpdate: Update, @@ -559,6 +591,12 @@ export function processUpdateQueue( queue.firstBaseUpdate = newFirstBaseUpdate; queue.lastBaseUpdate = newLastBaseUpdate; + if (firstBaseUpdate === null) { + // `queue.lanes` is used for entangling transitions. We can set it back to + // zero once the queue is empty. + queue.shared.lanes = NoLanes; + } + // Set the remaining expiration time to be whatever is remaining in the queue. // This should be fine because the only two other things that contribute to // expiration time are props and context. We're already in the middle of the