Skip to content

Commit

Permalink
Refactor of interleaved ("concurrent") update queue (#24663)
Browse files Browse the repository at this point in the history
* Always push updates to interleaved queue first

Interleaves updates (updates that are scheduled while another render
is already is progress) go into a special queue that isn't applied until
the end of the current render. They are transferred to the "real" queue
at the beginning of the next render.

Currently we check during `setState` whether an update should go
directly onto the real queue or onto the special interleaved queue. The
logic is subtle and it can lead to bugs if you mess it up, as in #24400.

Instead, this changes it to always go onto the interleaved queue. The
behavior is the same but the logic is simpler.

As a further step, we can also wait to update the `childLanes` until
the end of the current render. I'll do this in the next step.

* Move setState return path traversal to own call

A lot of the logic around scheduling an update needs access to the
fiber root. To obtain this reference, we must walk up the fiber return
path. We also do this to update `childLanes` on all the parent
nodes, so we can use the same traversal for both purposes.

The traversal currently happens inside `scheduleUpdateOnFiber`, but
sometimes we need to access it beyond that function, too.

So I've hoisted the traversal out of `scheduleUpdateOnFiber` into its
own function call that happens at the beginning of the
`setState` algorithm.

* Rename ReactInterleavedUpdates -> ReactFiberConcurrentUpdates

The scope of this module is expanding so I've renamed accordingly. No
behavioral changes.

* Enqueue and update childLanes in same function

During a setState, the childLanes are updated immediately, even if a
render is already in progress. This can lead to subtle concurrency bugs,
so the plan is to wait until the in-progress render has finished before
updating the childLanes, to prevent subtle concurrency bugs.

As a step toward that change, when scheduling an update, we should not
update the childLanes directly, but instead defer to the
ReactConcurrentUpdates module to do it at the appropriate time.

This makes markUpdateLaneFromFiberToRoot a private function that is
only called from the ReactConcurrentUpdates module.

* [FORKED] Don't update childLanes until after current render

(This is the riskiest commit in the stack. Only affects the "new"
reconciler fork.)

Updates that occur in a concurrent event while a render is already in
progress can't be processed during that render. This is tricky to get
right. Previously we solved this by adding concurrent updates to a
special `interleaved` queue, then transferring the `interleaved` queue
to the `pending` queue after the render phase had completed.

However, we would still mutate the `childLanes` along the parent path
immediately, which can lead to its own subtle data races.

Instead, we can queue the entire operation until after the render phase
has completed. This replaces the need for an `interleaved` field on
every fiber/hook queue.

The main motivation for this change, aside from simplifying the logic a
bit, is so we can read information about the current fiber while we're
walking up its return path, like whether it's inside a hidden tree.
(I haven't done anything like that in this commit, though.)

* Add 17691ac to forked revisions
  • Loading branch information
acdlite authored Jun 6, 2022
1 parent 4ddd8b4 commit 1cd90d2
Show file tree
Hide file tree
Showing 28 changed files with 729 additions and 562 deletions.
2 changes: 1 addition & 1 deletion packages/react-noop-renderer/src/createReactNoop.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import type {
Fiber,
TransitionTracingCallbacks,
} from 'react-reconciler/src/ReactInternalTypes';
import type {UpdateQueue} from 'react-reconciler/src/ReactUpdateQueue';
import type {UpdateQueue} from 'react-reconciler/src/ReactFiberClassUpdateQueue.new';
import type {ReactNodeList} from 'shared/ReactTypes';
import type {RootTag} from 'react-reconciler/src/ReactRootTags';

Expand Down
13 changes: 10 additions & 3 deletions packages/react-reconciler/src/ReactFiberBeginWork.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ import type {
CacheComponentState,
SpawnedCachePool,
} from './ReactFiberCacheComponent.new';
import type {UpdateQueue} from './ReactUpdateQueue.new';
import type {UpdateQueue} from './ReactFiberClassUpdateQueue.new';
import type {RootState} from './ReactFiberRoot.new';
import {
enableSuspenseAvoidThisFallback,
Expand Down Expand Up @@ -131,7 +131,7 @@ import {
cloneUpdateQueue,
initializeUpdateQueue,
enqueueCapturedUpdate,
} from './ReactUpdateQueue.new';
} from './ReactFiberClassUpdateQueue.new';
import {
NoLane,
NoLanes,
Expand Down Expand Up @@ -234,6 +234,7 @@ import {
getWorkInProgressRoot,
pushRenderLanes,
} from './ReactFiberWorkLoop.new';
import {enqueueConcurrentRenderForLane} from './ReactFiberConcurrentUpdates.new';
import {setWorkInProgressVersion} from './ReactMutableSource.new';
import {pushCacheProvider, CacheContext} from './ReactFiberCacheComponent.new';
import {createCapturedValue} from './ReactCapturedValue';
Expand Down Expand Up @@ -2626,7 +2627,13 @@ function updateDehydratedSuspenseComponent(
suspenseState.retryLane = attemptHydrationAtLane;
// TODO: Ideally this would inherit the event time of the current render
const eventTime = NoTimestamp;
scheduleUpdateOnFiber(current, attemptHydrationAtLane, eventTime);
enqueueConcurrentRenderForLane(current, attemptHydrationAtLane);
scheduleUpdateOnFiber(
root,
current,
attemptHydrationAtLane,
eventTime,
);
} else {
// We have already tried to ping at a higher priority than we're rendering with
// so if we got here, we must have failed to hydrate at those levels. We must
Expand Down
13 changes: 10 additions & 3 deletions packages/react-reconciler/src/ReactFiberBeginWork.old.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ import type {
CacheComponentState,
SpawnedCachePool,
} from './ReactFiberCacheComponent.old';
import type {UpdateQueue} from './ReactUpdateQueue.old';
import type {UpdateQueue} from './ReactFiberClassUpdateQueue.old';
import type {RootState} from './ReactFiberRoot.old';
import {
enableSuspenseAvoidThisFallback,
Expand Down Expand Up @@ -131,7 +131,7 @@ import {
cloneUpdateQueue,
initializeUpdateQueue,
enqueueCapturedUpdate,
} from './ReactUpdateQueue.old';
} from './ReactFiberClassUpdateQueue.old';
import {
NoLane,
NoLanes,
Expand Down Expand Up @@ -234,6 +234,7 @@ import {
getWorkInProgressRoot,
pushRenderLanes,
} from './ReactFiberWorkLoop.old';
import {enqueueConcurrentRenderForLane} from './ReactFiberConcurrentUpdates.old';
import {setWorkInProgressVersion} from './ReactMutableSource.old';
import {pushCacheProvider, CacheContext} from './ReactFiberCacheComponent.old';
import {createCapturedValue} from './ReactCapturedValue';
Expand Down Expand Up @@ -2626,7 +2627,13 @@ function updateDehydratedSuspenseComponent(
suspenseState.retryLane = attemptHydrationAtLane;
// TODO: Ideally this would inherit the event time of the current render
const eventTime = NoTimestamp;
scheduleUpdateOnFiber(current, attemptHydrationAtLane, eventTime);
enqueueConcurrentRenderForLane(current, attemptHydrationAtLane);
scheduleUpdateOnFiber(
root,
current,
attemptHydrationAtLane,
eventTime,
);
} else {
// We have already tried to ping at a higher priority than we're rendering with
// so if we got here, we must have failed to hydrate at those levels. We must
Expand Down
16 changes: 8 additions & 8 deletions packages/react-reconciler/src/ReactFiberClassComponent.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

import type {Fiber} from './ReactInternalTypes';
import type {Lanes} from './ReactFiberLane.new';
import type {UpdateQueue} from './ReactUpdateQueue.new';
import type {UpdateQueue} from './ReactFiberClassUpdateQueue.new';
import type {Flags} from './ReactFiberFlags';

import * as React from 'react';
Expand Down Expand Up @@ -58,7 +58,7 @@ import {
ForceUpdate,
initializeUpdateQueue,
cloneUpdateQueue,
} from './ReactUpdateQueue.new';
} from './ReactFiberClassUpdateQueue.new';
import {NoLanes} from './ReactFiberLane.new';
import {
cacheContext,
Expand Down Expand Up @@ -215,9 +215,9 @@ const classComponentUpdater = {
update.callback = callback;
}

enqueueUpdate(fiber, update, lane);
const root = scheduleUpdateOnFiber(fiber, lane, eventTime);
const root = enqueueUpdate(fiber, update, lane);
if (root !== null) {
scheduleUpdateOnFiber(root, fiber, lane, eventTime);
entangleTransitions(root, fiber, lane);
}

Expand Down Expand Up @@ -250,9 +250,9 @@ const classComponentUpdater = {
update.callback = callback;
}

enqueueUpdate(fiber, update, lane);
const root = scheduleUpdateOnFiber(fiber, lane, eventTime);
const root = enqueueUpdate(fiber, update, lane);
if (root !== null) {
scheduleUpdateOnFiber(root, fiber, lane, eventTime);
entangleTransitions(root, fiber, lane);
}

Expand Down Expand Up @@ -284,9 +284,9 @@ const classComponentUpdater = {
update.callback = callback;
}

enqueueUpdate(fiber, update, lane);
const root = scheduleUpdateOnFiber(fiber, lane, eventTime);
const root = enqueueUpdate(fiber, update, lane);
if (root !== null) {
scheduleUpdateOnFiber(root, fiber, lane, eventTime);
entangleTransitions(root, fiber, lane);
}

Expand Down
16 changes: 8 additions & 8 deletions packages/react-reconciler/src/ReactFiberClassComponent.old.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

import type {Fiber} from './ReactInternalTypes';
import type {Lanes} from './ReactFiberLane.old';
import type {UpdateQueue} from './ReactUpdateQueue.old';
import type {UpdateQueue} from './ReactFiberClassUpdateQueue.old';
import type {Flags} from './ReactFiberFlags';

import * as React from 'react';
Expand Down Expand Up @@ -58,7 +58,7 @@ import {
ForceUpdate,
initializeUpdateQueue,
cloneUpdateQueue,
} from './ReactUpdateQueue.old';
} from './ReactFiberClassUpdateQueue.old';
import {NoLanes} from './ReactFiberLane.old';
import {
cacheContext,
Expand Down Expand Up @@ -215,9 +215,9 @@ const classComponentUpdater = {
update.callback = callback;
}

enqueueUpdate(fiber, update, lane);
const root = scheduleUpdateOnFiber(fiber, lane, eventTime);
const root = enqueueUpdate(fiber, update, lane);
if (root !== null) {
scheduleUpdateOnFiber(root, fiber, lane, eventTime);
entangleTransitions(root, fiber, lane);
}

Expand Down Expand Up @@ -250,9 +250,9 @@ const classComponentUpdater = {
update.callback = callback;
}

enqueueUpdate(fiber, update, lane);
const root = scheduleUpdateOnFiber(fiber, lane, eventTime);
const root = enqueueUpdate(fiber, update, lane);
if (root !== null) {
scheduleUpdateOnFiber(root, fiber, lane, eventTime);
entangleTransitions(root, fiber, lane);
}

Expand Down Expand Up @@ -284,9 +284,9 @@ const classComponentUpdater = {
update.callback = callback;
}

enqueueUpdate(fiber, update, lane);
const root = scheduleUpdateOnFiber(fiber, lane, eventTime);
const root = enqueueUpdate(fiber, update, lane);
if (root !== null) {
scheduleUpdateOnFiber(root, fiber, lane, eventTime);
entangleTransitions(root, fiber, lane);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,9 +107,12 @@ import {debugRenderPhaseSideEffectsForStrictMode} from 'shared/ReactFeatureFlags
import {StrictLegacyMode} from './ReactTypeOfMode';
import {
markSkippedUpdateLanes,
isInterleavedUpdate,
isUnsafeClassRenderPhaseUpdate,
} from './ReactFiberWorkLoop.new';
import {pushInterleavedQueue} from './ReactFiberInterleavedUpdates.new';
import {
enqueueConcurrentClassUpdate,
unsafe_markUpdateLaneFromFiberToRoot,
} from './ReactFiberConcurrentUpdates.new';
import {setIsStrictModeForDevtools} from './ReactFiberDevToolsHook.new';

import assign from 'shared/assign';
Expand All @@ -129,7 +132,6 @@ export type Update<State> = {|

export type SharedQueue<State> = {|
pending: Update<State> | null,
interleaved: Update<State> | null,
lanes: Lanes,
|};

Expand Down Expand Up @@ -169,7 +171,6 @@ export function initializeUpdateQueue<State>(fiber: Fiber): void {
lastBaseUpdate: null,
shared: {
pending: null,
interleaved: null,
lanes: NoLanes,
},
effects: null,
Expand Down Expand Up @@ -214,40 +215,15 @@ export function enqueueUpdate<State>(
fiber: Fiber,
update: Update<State>,
lane: Lane,
) {
): FiberRoot | null {
const updateQueue = fiber.updateQueue;
if (updateQueue === null) {
// Only occurs if the fiber has been unmounted.
return;
return null;
}

const sharedQueue: SharedQueue<State> = (updateQueue: any).shared;

if (isInterleavedUpdate(fiber, lane)) {
const interleaved = sharedQueue.interleaved;
if (interleaved === null) {
// This is the first update. Create a circular list.
update.next = update;
// At the end of the current render, this queue's interleaved updates will
// be transferred to the pending queue.
pushInterleavedQueue(sharedQueue);
} else {
update.next = interleaved.next;
interleaved.next = update;
}
sharedQueue.interleaved = update;
} else {
const pending = sharedQueue.pending;
if (pending === null) {
// This is the first update. Create a circular list.
update.next = update;
} else {
update.next = pending.next;
pending.next = update;
}
sharedQueue.pending = update;
}

if (__DEV__) {
if (
currentlyProcessingQueue === sharedQueue &&
Expand All @@ -262,6 +238,28 @@ export function enqueueUpdate<State>(
didWarnUpdateInsideUpdate = true;
}
}

if (isUnsafeClassRenderPhaseUpdate(fiber)) {
// This is an unsafe render phase update. Add directly to the update
// queue so we can process it immediately during the current render.
const pending = sharedQueue.pending;
if (pending === null) {
// This is the first update. Create a circular list.
update.next = update;
} else {
update.next = pending.next;
pending.next = update;
}
sharedQueue.pending = update;

// Update the childLanes even though we're most likely already rendering
// this fiber. This is for backwards compatibility in the case where you
// update a different component during render phase than the one that is
// currently renderings (a pattern that is accompanied by a warning).
return unsafe_markUpdateLaneFromFiberToRoot(fiber, lane);
} else {
return enqueueConcurrentClassUpdate(fiber, sharedQueue, update, lane);
}
}

export function entangleTransitions(root: FiberRoot, fiber: Fiber, lane: Lane) {
Expand Down Expand Up @@ -622,17 +620,7 @@ export function processUpdateQueue<State>(
queue.firstBaseUpdate = newFirstBaseUpdate;
queue.lastBaseUpdate = newLastBaseUpdate;

// Interleaved updates are stored on a separate queue. We aren't going to
// process them during this render, but we do need to track which lanes
// are remaining.
const lastInterleaved = queue.shared.interleaved;
if (lastInterleaved !== null) {
let interleaved = lastInterleaved;
do {
newLanes = mergeLanes(newLanes, interleaved.lane);
interleaved = ((interleaved: any).next: Update<State>);
} while (interleaved !== lastInterleaved);
} else if (firstBaseUpdate === null) {
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;
Expand Down
Loading

0 comments on commit 1cd90d2

Please sign in to comment.