Assign different lanes to consecutive transitions

Currently we always assign the same lane to all transitions. This means
if there are two pending transitions at the same time, neither
transition can finish until both can finish, even if they affect
completely separate parts of the UI.

The new approach is to assign a different lane to each consecutive
transition, by shifting the bit to the right each time. When we reach
the end of the bit range, we cycle back to the first bit. In practice,
this should mean that all transitions get their own dedicated lane,
unless we have more pending transitions than lanes, which should
be rare.

We retain our existing behavior of assigning the same lane to all
transitions within the same event. This is achieved by caching the first
lane assigned to a transition, then re-using that one until the next
React task, by which point the event must have finished. This preserves
the guarantee that all transition updates that result from a single
event will be consistent.

Author: acdlite

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

@@ -92,6 +92,7 @@ export const DefaultLanes: Lanes = /*                   */ 0b0000000000000000000
 
 const TransitionHydrationLane: Lane = /*                */ 0b0000000000000000001000000000000;
 const TransitionLanes: Lanes = /*                       */ 0b0000000001111111110000000000000;
+const SomeTransitionLane: Lane = /*                     */ 0b0000000000000000010000000000000;
 
 const RetryLanes: Lanes = /*                            */ 0b0000011110000000000000000000000;
 
@@ -110,6 +111,9 @@ export const NoTimestamp = -1;
 
 let currentUpdateLanePriority: LanePriority = NoLanePriority;
 
+let nextTransitionLane: Lane = SomeTransitionLane;
+let nextRetryLane: Lane = SomeRetryLane;
+
 export function getCurrentUpdateLanePriority(): LanePriority {
   return currentUpdateLanePriority;
 }
@@ -338,6 +342,11 @@ export function getNextLanes(root: FiberRoot, wipLanes: Lanes): Lanes {
   // entanglement is usually "best effort": we'll try our best to render the
   // lanes in the same batch, but it's not worth throwing out partially
   // completed work in order to do it.
+  // TODO: Reconsider this. The counter-argument is that the partial work
+  // represents an intermediate state, which we don't want to show to the user.
+  // And by spending extra time finishing it, we're increasing the amount of
+  // time it takes to show the final state, which is what they are actually
+  // waiting for.
   //
   // For those exceptions where entanglement is semantically important, like
   // useMutableSource, we should ensure that there is no partial work at the
@@ -547,34 +556,23 @@ export function findUpdateLane(
   );
 }
 
-// To ensure consistency across multiple updates in the same event, this should
-// be pure function, so that it always returns the same lane for given inputs.
-export function findTransitionLane(wipLanes: Lanes, pendingLanes: Lanes): Lane {
-  // First look for lanes that are completely unclaimed, i.e. have no
-  // pending work.
-  let lane = pickArbitraryLane(TransitionLanes & ~pendingLanes);
-  if (lane === NoLane) {
-    // If all lanes have pending work, look for a lane that isn't currently
-    // being worked on.
-    lane = pickArbitraryLane(TransitionLanes & ~wipLanes);
-    if (lane === NoLane) {
-      // If everything is being worked on, pick any lane. This has the
-      // effect of interrupting the current work-in-progress.
-      lane = pickArbitraryLane(TransitionLanes);
-    }
+export function claimNextTransitionLane(): Lane {
+  // Cycle through the lanes, assigning each new transition to the next lane.
+  // In most cases, this means every transition gets its own lane, until we
+  // run out of lanes and cycle back to the beginning.
+  const lane = nextTransitionLane;
+  nextTransitionLane <<= 1;
+  if ((nextTransitionLane & TransitionLanes) === 0) {
+    nextTransitionLane = SomeTransitionLane;
   }
   return lane;
 }
 
-// To ensure consistency across multiple updates in the same event, this should
-// be pure function, so that it always returns the same lane for given inputs.
-export function findRetryLane(wipLanes: Lanes): Lane {
-  // This is a fork of `findUpdateLane` designed specifically for Suspense
-  // "retries" — a special update that attempts to flip a Suspense boundary
-  // from its placeholder state to its primary/resolved state.
-  let lane = pickArbitraryLane(RetryLanes & ~wipLanes);
-  if (lane === NoLane) {
-    lane = pickArbitraryLane(RetryLanes);
+export function claimNextRetryLane(): Lane {
+  const lane = nextRetryLane;
+  nextRetryLane <<= 1;
+  if ((nextRetryLane & RetryLanes) === 0) {
+    nextRetryLane = SomeRetryLane;
   }
   return lane;
 }
@@ -761,16 +759,32 @@ export function markRootFinished(root: FiberRoot, remainingLanes: Lanes) {
 }
 
 export function markRootEntangled(root: FiberRoot, entangledLanes: Lanes) {
-  root.entangledLanes |= entangledLanes;
+  // In addition to entangling each of the given lanes with each other, we also
+  // have to consider _transitive_ entanglements. For each lane that is already
+  // entangled with *any* of the given lanes, that lane is now transitively
+  // entangled with *all* the given lanes.
+  //
+  // Translated: If C is entangled with A, then entangling A with B also
+  // entangles C with B.
+  //
+  // If this is hard to grasp, it might help to intentionally break this
+  // function and look at the tests that fail in ReactTransition-test.js. Try
+  // commenting out one of the conditions below.
 
+  const rootEntangledLanes = (root.entangledLanes |= entangledLanes);
   const entanglements = root.entanglements;
-  let lanes = entangledLanes;
-  while (lanes > 0) {
+  let lanes = rootEntangledLanes;
+  while (lanes) {
     const index = pickArbitraryLaneIndex(lanes);
     const lane = 1 << index;
-
-    entanglements[index] |= entangledLanes;
-
+    if (
+      // Is this one of the newly entangled lanes?
+      (lane & entangledLanes) |
+      // Is this lane transitively entangled with the newly entangled lanes?
+      (entanglements[index] & entangledLanes)
+    ) {
+      entanglements[index] |= entangledLanes;
+    }
     lanes &= ~lane;
   }
 }

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

@@ -92,6 +92,7 @@ export const DefaultLanes: Lanes = /*                   */ 0b0000000000000000000
 
 const TransitionHydrationLane: Lane = /*                */ 0b0000000000000000001000000000000;
 const TransitionLanes: Lanes = /*                       */ 0b0000000001111111110000000000000;
+const SomeTransitionLane: Lane = /*                     */ 0b0000000000000000010000000000000;
 
 const RetryLanes: Lanes = /*                            */ 0b0000011110000000000000000000000;
 
@@ -110,6 +111,9 @@ export const NoTimestamp = -1;
 
 let currentUpdateLanePriority: LanePriority = NoLanePriority;
 
+let nextTransitionLane: Lane = SomeTransitionLane;
+let nextRetryLane: Lane = SomeRetryLane;
+
 export function getCurrentUpdateLanePriority(): LanePriority {
   return currentUpdateLanePriority;
 }
@@ -338,6 +342,11 @@ export function getNextLanes(root: FiberRoot, wipLanes: Lanes): Lanes {
   // entanglement is usually "best effort": we'll try our best to render the
   // lanes in the same batch, but it's not worth throwing out partially
   // completed work in order to do it.
+  // TODO: Reconsider this. The counter-argument is that the partial work
+  // represents an intermediate state, which we don't want to show to the user.
+  // And by spending extra time finishing it, we're increasing the amount of
+  // time it takes to show the final state, which is what they are actually
+  // waiting for.
   //
   // For those exceptions where entanglement is semantically important, like
   // useMutableSource, we should ensure that there is no partial work at the
@@ -547,34 +556,23 @@ export function findUpdateLane(
   );
 }
 
-// To ensure consistency across multiple updates in the same event, this should
-// be pure function, so that it always returns the same lane for given inputs.
-export function findTransitionLane(wipLanes: Lanes, pendingLanes: Lanes): Lane {
-  // First look for lanes that are completely unclaimed, i.e. have no
-  // pending work.
-  let lane = pickArbitraryLane(TransitionLanes & ~pendingLanes);
-  if (lane === NoLane) {
-    // If all lanes have pending work, look for a lane that isn't currently
-    // being worked on.
-    lane = pickArbitraryLane(TransitionLanes & ~wipLanes);
-    if (lane === NoLane) {
-      // If everything is being worked on, pick any lane. This has the
-      // effect of interrupting the current work-in-progress.
-      lane = pickArbitraryLane(TransitionLanes);
-    }
+export function claimNextTransitionLane(): Lane {
+  // Cycle through the lanes, assigning each new transition to the next lane.
+  // In most cases, this means every transition gets its own lane, until we
+  // run out of lanes and cycle back to the beginning.
+  const lane = nextTransitionLane;
+  nextTransitionLane <<= 1;
+  if ((nextTransitionLane & TransitionLanes) === 0) {
+    nextTransitionLane = SomeTransitionLane;
   }
   return lane;
 }
 
-// To ensure consistency across multiple updates in the same event, this should
-// be pure function, so that it always returns the same lane for given inputs.
-export function findRetryLane(wipLanes: Lanes): Lane {
-  // This is a fork of `findUpdateLane` designed specifically for Suspense
-  // "retries" — a special update that attempts to flip a Suspense boundary
-  // from its placeholder state to its primary/resolved state.
-  let lane = pickArbitraryLane(RetryLanes & ~wipLanes);
-  if (lane === NoLane) {
-    lane = pickArbitraryLane(RetryLanes);
+export function claimNextRetryLane(): Lane {
+  const lane = nextRetryLane;
+  nextRetryLane <<= 1;
+  if ((nextRetryLane & RetryLanes) === 0) {
+    nextRetryLane = SomeRetryLane;
   }
   return lane;
 }
@@ -761,16 +759,32 @@ export function markRootFinished(root: FiberRoot, remainingLanes: Lanes) {
 }
 
 export function markRootEntangled(root: FiberRoot, entangledLanes: Lanes) {
-  root.entangledLanes |= entangledLanes;
+  // In addition to entangling each of the given lanes with each other, we also
+  // have to consider _transitive_ entanglements. For each lane that is already
+  // entangled with *any* of the given lanes, that lane is now transitively
+  // entangled with *all* the given lanes.
+  //
+  // Translated: If C is entangled with A, then entangling A with B also
+  // entangles C with B.
+  //
+  // If this is hard to grasp, it might help to intentionally break this
+  // function and look at the tests that fail in ReactTransition-test.js. Try
+  // commenting out one of the conditions below.
 
+  const rootEntangledLanes = (root.entangledLanes |= entangledLanes);
   const entanglements = root.entanglements;
-  let lanes = entangledLanes;
-  while (lanes > 0) {
+  let lanes = rootEntangledLanes;
+  while (lanes) {
     const index = pickArbitraryLaneIndex(lanes);
     const lane = 1 << index;
-
-    entanglements[index] |= entangledLanes;
-
+    if (
+      // Is this one of the newly entangled lanes?
+      (lane & entangledLanes) |
+      // Is this lane transitively entangled with the newly entangled lanes?
+      (entanglements[index] & entangledLanes)
+    ) {
+      entanglements[index] |= entangledLanes;
+    }
     lanes &= ~lane;
   }
 }

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

@@ -145,8 +145,8 @@ import {
   SyncBatchedLane,
   NoTimestamp,
   findUpdateLane,
-  findTransitionLane,
-  findRetryLane,
+  claimNextTransitionLane,
+  claimNextRetryLane,
   includesSomeLane,
   isSubsetOfLanes,
   mergeLanes,
@@ -302,8 +302,6 @@ let workInProgressRootUpdatedLanes: Lanes = NoLanes;
 // Lanes that were pinged (in an interleaved event) during this render.
 let workInProgressRootPingedLanes: Lanes = NoLanes;
 
-let mostRecentlyUpdatedRoot: FiberRoot | null = null;
-
 // The most recent time we committed a fallback. This lets us ensure a train
 // model where we don't commit new loading states in too quick succession.
 let globalMostRecentFallbackTime: number = 0;
@@ -360,7 +358,7 @@ let spawnedWorkDuringRender: null | Array<Lane | Lanes> = null;
 // between the first and second call.
 let currentEventTime: number = NoTimestamp;
 let currentEventWipLanes: Lanes = NoLanes;
-let currentEventPendingLanes: Lanes = NoLanes;
+let currentEventTransitionLane: Lanes = NoLanes;
 
 // Dev only flag that tracks if passive effects are currently being flushed.
 // We warn about state updates for unmounted components differently in this case.
@@ -428,20 +426,17 @@ export function requestUpdateLane(fiber: Fiber): Lane {
   // event. Then reset the cached values once we can be sure the event is over.
   // Our heuristic for that is whenever we enter a concurrent work loop.
   //
-  // We'll do the same for `currentEventPendingLanes` below.
+  // We'll do the same for `currentEventTransitionLane` below.
   if (currentEventWipLanes === NoLanes) {
     currentEventWipLanes = workInProgressRootIncludedLanes;
   }
 
   const isTransition = requestCurrentTransition() !== NoTransition;
   if (isTransition) {
-    if (currentEventPendingLanes !== NoLanes) {
-      currentEventPendingLanes =
-        mostRecentlyUpdatedRoot !== null
-          ? mostRecentlyUpdatedRoot.pendingLanes
-          : NoLanes;
+    if (currentEventTransitionLane === NoLane) {
+      currentEventTransitionLane = claimNextTransitionLane();
     }
-    return findTransitionLane(currentEventWipLanes, currentEventPendingLanes);
+    return currentEventTransitionLane;
   }
 
   // TODO: Remove this dependency on the Scheduler priority.
@@ -494,7 +489,8 @@ function requestRetryLane(fiber: Fiber) {
   if (currentEventWipLanes === NoLanes) {
     currentEventWipLanes = workInProgressRootIncludedLanes;
   }
-  return findRetryLane(currentEventWipLanes);
+
+  return claimNextRetryLane();
 }
 
 export function scheduleUpdateOnFiber(
@@ -618,13 +614,6 @@ export function scheduleUpdateOnFiber(
     schedulePendingInteractions(root, lane);
   }
 
-  // We use this when assigning a lane for a transition inside
-  // `requestUpdateLane`. We assume it's the same as the root being updated,
-  // since in the common case of a single root app it probably is. If it's not
-  // the same root, then it's not a huge deal, we just might batch more stuff
-  // together more than necessary.
-  mostRecentlyUpdatedRoot = root;
-
   return root;
 }
 
@@ -793,7 +782,7 @@ function performConcurrentWorkOnRoot(root, didTimeout) {
   // event time. The next update will compute a new event time.
   currentEventTime = NoTimestamp;
   currentEventWipLanes = NoLanes;
-  currentEventPendingLanes = NoLanes;
+  currentEventTransitionLane = NoLanes;
 
   invariant(
     (executionContext & (RenderContext | CommitContext)) === NoContext,
@@ -2461,6 +2450,8 @@ function retryTimedOutBoundary(boundaryFiber: Fiber, retryLane: Lane) {
   // suspended it has resolved, which means at least part of the tree was
   // likely unblocked. Try rendering again, at a new expiration time.
   if (retryLane === NoLane) {
+    // TODO: Assign this to `suspenseState.retryLane`? to avoid
+    // unnecessary entanglement?
     retryLane = requestRetryLane(boundaryFiber);
   }
   // TODO: Special case idle priority?