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

Refactor of interleaved ("concurrent") update queue #24663

Merged
merged 6 commits into from
Jun 6, 2022
Merged

Refactor of interleaved ("concurrent") update queue #24663

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
7ea43a8
Always push updates to interleaved queue first
acdlite May 31, 2022
05023aa
Move setState return path traversal to own call
acdlite May 31, 2022
9b33a5e
Rename ReactInterleavedUpdates -> ReactFiberConcurrentUpdates
acdlite May 31, 2022
bd04069
Enqueue and update childLanes in same function
acdlite Jun 1, 2022
17691ac
[FORKED] Don't update childLanes until after current render
acdlite Jun 2, 2022
f5b0535
Add 17691ac to forked revisions
acdlite Jun 6, 2022
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
Prev Previous commit
Next Next commit
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.
  • Loading branch information
@acdlite
acdlite committed Jun 6, 2022
commit bd04069f89866dd6b9741b2afd7a4445ce8e0441
4 changes: 2 additions & 2 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 @@ -228,13 +228,13 @@ import {
} from './ReactFiber.new';
import {
retryDehydratedSuspenseBoundary,
markUpdateLaneFromFiberToRoot,
scheduleUpdateOnFiber,
renderDidSuspendDelayIfPossible,
markSkippedUpdateLanes,
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 @@ -2627,7 +2627,7 @@ function updateDehydratedSuspenseComponent(
suspenseState.retryLane = attemptHydrationAtLane;
// TODO: Ideally this would inherit the event time of the current render
const eventTime = NoTimestamp;
markUpdateLaneFromFiberToRoot(current, attemptHydrationAtLane);
enqueueConcurrentRenderForLane(current, attemptHydrationAtLane);
scheduleUpdateOnFiber(
root,
current,
Expand Down
4 changes: 2 additions & 2 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 @@ -228,13 +228,13 @@ import {
} from './ReactFiber.old';
import {
retryDehydratedSuspenseBoundary,
markUpdateLaneFromFiberToRoot,
scheduleUpdateOnFiber,
renderDidSuspendDelayIfPossible,
markSkippedUpdateLanes,
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 @@ -2627,7 +2627,7 @@ function updateDehydratedSuspenseComponent(
suspenseState.retryLane = attemptHydrationAtLane;
// TODO: Ideally this would inherit the event time of the current render
const eventTime = NoTimestamp;
markUpdateLaneFromFiberToRoot(current, attemptHydrationAtLane);
enqueueConcurrentRenderForLane(current, attemptHydrationAtLane);
scheduleUpdateOnFiber(
root,
current,
Expand Down
10 changes: 3 additions & 7 deletions packages/react-reconciler/src/ReactFiberClassComponent.new.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@ import {
requestEventTime,
requestUpdateLane,
scheduleUpdateOnFiber,
markUpdateLaneFromFiberToRoot,
} from './ReactFiberWorkLoop.new';
import {logForceUpdateScheduled, logStateUpdateScheduled} from './DebugTracing';
import {
Expand Down Expand Up @@ -216,8 +215,7 @@ const classComponentUpdater = {
update.callback = callback;
}

enqueueUpdate(fiber, update, lane);
const root = markUpdateLaneFromFiberToRoot(fiber, lane);
const root = enqueueUpdate(fiber, update, lane);
if (root !== null) {
scheduleUpdateOnFiber(root, fiber, lane, eventTime);
Copy link
Member

Choose a reason for hiding this comment

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

Hoisting this up inside of this check will skip checkForNestedUpdates and the insertion effect in render warning. What cases would make it possible to skip those now due to root being null?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It would only happen when the component is unmounted. Those checks should probably move to enqueueUpdate, though, since that's where similar warnings are.

entangleTransitions(root, fiber, lane);
Expand Down Expand Up @@ -252,8 +250,7 @@ const classComponentUpdater = {
update.callback = callback;
}

enqueueUpdate(fiber, update, lane);
const root = markUpdateLaneFromFiberToRoot(fiber, lane);
const root = enqueueUpdate(fiber, update, lane);
if (root !== null) {
scheduleUpdateOnFiber(root, fiber, lane, eventTime);
entangleTransitions(root, fiber, lane);
Expand Down Expand Up @@ -287,8 +284,7 @@ const classComponentUpdater = {
update.callback = callback;
}

enqueueUpdate(fiber, update, lane);
const root = markUpdateLaneFromFiberToRoot(fiber, lane);
const root = enqueueUpdate(fiber, update, lane);
if (root !== null) {
scheduleUpdateOnFiber(root, fiber, lane, eventTime);
entangleTransitions(root, fiber, lane);
Expand Down
10 changes: 3 additions & 7 deletions packages/react-reconciler/src/ReactFiberClassComponent.old.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@ import {
requestEventTime,
requestUpdateLane,
scheduleUpdateOnFiber,
markUpdateLaneFromFiberToRoot,
} from './ReactFiberWorkLoop.old';
import {logForceUpdateScheduled, logStateUpdateScheduled} from './DebugTracing';
import {
Expand Down Expand Up @@ -216,8 +215,7 @@ const classComponentUpdater = {
update.callback = callback;
}

enqueueUpdate(fiber, update, lane);
const root = markUpdateLaneFromFiberToRoot(fiber, lane);
const root = enqueueUpdate(fiber, update, lane);
if (root !== null) {
scheduleUpdateOnFiber(root, fiber, lane, eventTime);
entangleTransitions(root, fiber, lane);
Expand Down Expand Up @@ -252,8 +250,7 @@ const classComponentUpdater = {
update.callback = callback;
}

enqueueUpdate(fiber, update, lane);
const root = markUpdateLaneFromFiberToRoot(fiber, lane);
const root = enqueueUpdate(fiber, update, lane);
if (root !== null) {
scheduleUpdateOnFiber(root, fiber, lane, eventTime);
entangleTransitions(root, fiber, lane);
Expand Down Expand Up @@ -287,8 +284,7 @@ const classComponentUpdater = {
update.callback = callback;
}

enqueueUpdate(fiber, update, lane);
const root = markUpdateLaneFromFiberToRoot(fiber, lane);
const root = enqueueUpdate(fiber, update, lane);
if (root !== null) {
scheduleUpdateOnFiber(root, fiber, lane, eventTime);
entangleTransitions(root, fiber, lane);
Expand Down
58 changes: 28 additions & 30 deletions packages/react-reconciler/src/ReactFiberClassUpdateQueue.new.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,10 @@ import {
markSkippedUpdateLanes,
isUnsafeClassRenderPhaseUpdate,
} from './ReactFiberWorkLoop.new';
import {pushConcurrentUpdateQueue} from './ReactFiberConcurrentUpdates.new';
import {
enqueueConcurrentClassUpdate,
unsafe_markUpdateLaneFromFiberToRoot,
} from './ReactFiberConcurrentUpdates.new';
import {setIsStrictModeForDevtools} from './ReactFiberDevToolsHook.new';

import assign from 'shared/assign';
Expand Down Expand Up @@ -214,42 +217,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 (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;
} else {
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.
pushConcurrentUpdateQueue(sharedQueue);
} else {
update.next = interleaved.next;
interleaved.next = update;
}
sharedQueue.interleaved = update;
}

if (__DEV__) {
if (
currentlyProcessingQueue === sharedQueue &&
Expand All @@ -264,6 +240,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
58 changes: 28 additions & 30 deletions packages/react-reconciler/src/ReactFiberClassUpdateQueue.old.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,10 @@ import {
markSkippedUpdateLanes,
isUnsafeClassRenderPhaseUpdate,
} from './ReactFiberWorkLoop.old';
import {pushConcurrentUpdateQueue} from './ReactFiberConcurrentUpdates.old';
import {
enqueueConcurrentClassUpdate,
unsafe_markUpdateLaneFromFiberToRoot,
} from './ReactFiberConcurrentUpdates.old';
import {setIsStrictModeForDevtools} from './ReactFiberDevToolsHook.old';

import assign from 'shared/assign';
Expand Down Expand Up @@ -214,42 +217,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 (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;
} else {
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.
pushConcurrentUpdateQueue(sharedQueue);
} else {
update.next = interleaved.next;
interleaved.next = update;
}
sharedQueue.interleaved = update;
}

if (__DEV__) {
if (
currentlyProcessingQueue === sharedQueue &&
Expand All @@ -264,6 +240,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
Loading