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

Extract queueing logic into shared functions #22452

Merged
merged 1 commit into from
Sep 28, 2021
Merged

Extract queueing logic into shared functions #22452

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
246 changes: 107 additions & 139 deletions packages/react-reconciler/src/ReactFiberHooks.new.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,9 +113,9 @@ import {logStateUpdateScheduled} from './DebugTracing';
import {markStateUpdateScheduled} from './SchedulingProfiler';
import {CacheContext} from './ReactFiberCacheComponent.new';
import {
createUpdate,
enqueueUpdate,
entangleTransitions,
createUpdate as createLegacyQueueUpdate,
enqueueUpdate as enqueueLegacyQueueUpdate,
entangleTransitions as entangleLegacyQueueTransitions,
} from './ReactUpdateQueue.new';
import {pushInterleavedQueue} from './ReactFiberInterleavedUpdates.new';
import {warnOnSubscriptionInsideStartTransition} from 'shared/ReactFeatureFlags';
Expand Down Expand Up @@ -2125,7 +2125,7 @@ function refreshCache<T>(fiber: Fiber, seedKey: ?() => T, seedValue: T) {
const eventTime = requestEventTime();
const root = scheduleUpdateOnFiber(provider, lane, eventTime);
if (root !== null) {
entangleTransitions(root, provider, lane);
entangleLegacyQueueTransitions(root, provider, lane);
}

const seededCache = new Map();
Expand All @@ -2136,12 +2136,12 @@ function refreshCache<T>(fiber: Fiber, seedKey: ?() => T, seedValue: T) {
}

// Schedule an update on the cache boundary to trigger a refresh.
const refreshUpdate = createUpdate(eventTime, lane);
const refreshUpdate = createLegacyQueueUpdate(eventTime, lane);
const payload = {
cache: seededCache,
};
refreshUpdate.payload = payload;
enqueueUpdate(provider, refreshUpdate, lane);
enqueueLegacyQueueUpdate(provider, refreshUpdate, lane);
return;
}
}
Expand All @@ -2165,7 +2165,6 @@ function dispatchReducerAction<S, A>(
}
}

const eventTime = requestEventTime();
const lane = requestUpdateLane(fiber);

const update: Update<S, A> = {
Expand All @@ -2176,90 +2175,25 @@ function dispatchReducerAction<S, A>(
next: (null: any),
};

const alternate = fiber.alternate;
if (
fiber === currentlyRenderingFiber ||
(alternate !== null && alternate === currentlyRenderingFiber)
) {
// This is a render phase update. Stash it in a lazily-created map of
// queue -> linked list of updates. After this render pass, we'll restart
// and apply the stashed updates on top of the work-in-progress hook.
didScheduleRenderPhaseUpdateDuringThisPass = didScheduleRenderPhaseUpdate = true;
const pending = queue.pending;
if (pending === null) {
// This is the first update. Create a circular list.
update.next = update;
} else {
update.next = pending.next;
pending.next = update;
}
queue.pending = update;
if (isRenderPhaseUpdate(fiber)) {
enqueueRenderPhaseUpdate(queue, update);
} else {
if (isInterleavedUpdate(fiber, lane)) {
const interleaved = queue.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(queue);
} else {
update.next = interleaved.next;
interleaved.next = update;
}
queue.interleaved = update;
} else {
const pending = queue.pending;
if (pending === null) {
// This is the first update. Create a circular list.
update.next = update;
} else {
update.next = pending.next;
pending.next = update;
}
queue.pending = update;
}
enqueueUpdate(fiber, queue, update, lane);

if (__DEV__) {
// $FlowExpectedError - jest isn't a global, and isn't recognized outside of tests
if ('undefined' !== typeof jest) {
warnIfNotCurrentlyActingUpdatesInDev(fiber);
}
}
const eventTime = requestEventTime();
Copy link
Contributor

Choose a reason for hiding this comment

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

i guess not that much work happens between where this call was and where it moved to, so the timing is still accurate enough?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah it doesn't need to be super precise, it's only used for starvation detection.

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);
queue.lanes = newQueueLanes;
// Even if queue.lanes already include lane, we don't know for certain if
// the lane finished since the last time we entangled it. So we need to
// entangle it again, just to be sure.
markRootEntangled(root, newQueueLanes);
}
}

if (__DEV__) {
if (enableDebugTracing) {
if (fiber.mode & DebugTracingMode) {
const name = getComponentNameFromFiber(fiber) || 'Unknown';
logStateUpdateScheduled(name, lane, action);
}
if (root !== null) {
entangleTransitionUpdate(root, queue, lane);
Comment on lines +2191 to +2192
Copy link
Contributor

Choose a reason for hiding this comment

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

One suggestion here: since entangleTransitionUpdate() immediately checks isTransitionLane(lane), it might be better to put the check outside and avoid calling the function unless isTransitionLane(lane) is true. Something like:

// Takes a lane, checks if its entangled, returns the lane as an opaque type wrapper if so
const entangledLane: EntangledLane | null = checkEntangledLane(lane);
if (entangedLand !== null) {
  // type of entangledLane is 
  // opaque type EntangledLane: Lane = Lane;
  entangleTransitionUpdate(root, queue, entangledLane);
}

This doesn't incur any overhead - entangedLane is still a number - but it uses types to enforce correct calling sequence. This also lets the JIT inline just checkEntangledLane(), and potentially avoid inlining the more rarely called entangleTransitionUpdate().

Maybe not worth it but figured i'd mention it.

Copy link
Collaborator Author

@acdlite acdlite Sep 28, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Lane used to be an opaque type but I had to un-opaque it because it leaks outside of the reconciler via the Fiber type, unfortunately. And the Fiber type is shared between reconciler forks, so we would have had to fork literally every file in our repo (i.e. React DOM, too) which I was loath to do. Our Flow types for Fiber are kind of a mess, not even sure if it's possible to model correctly and if it were it would involve so much work that we might as well just go all the way to Rust.

Re: avoiding the function call, Closure inlines both of these functions anyway, but that's a good point. I had it that way at first but I moved it for symmetry with the equivalent class update queue function. I figure I'll hoist it out as soon as we need to put additional transition-only logic in there.

}
}

if (enableSchedulingProfiler) {
markStateUpdateScheduled(fiber, lane);
}
markUpdateInDevTools(fiber, lane, action);
}

function dispatchSetState<S, A>(
Expand All @@ -2277,7 +2211,6 @@ function dispatchSetState<S, A>(
}
}

const eventTime = requestEventTime();
const lane = requestUpdateLane(fiber);

const update: Update<S, A> = {
Expand All @@ -2288,50 +2221,12 @@ function dispatchSetState<S, A>(
next: (null: any),
};

const alternate = fiber.alternate;
if (
fiber === currentlyRenderingFiber ||
(alternate !== null && alternate === currentlyRenderingFiber)
) {
// This is a render phase update. Stash it in a lazily-created map of
// queue -> linked list of updates. After this render pass, we'll restart
// and apply the stashed updates on top of the work-in-progress hook.
didScheduleRenderPhaseUpdateDuringThisPass = didScheduleRenderPhaseUpdate = true;
const pending = queue.pending;
if (pending === null) {
// This is the first update. Create a circular list.
update.next = update;
} else {
update.next = pending.next;
pending.next = update;
}
queue.pending = update;
if (isRenderPhaseUpdate(fiber)) {
enqueueRenderPhaseUpdate(queue, update);
} else {
if (isInterleavedUpdate(fiber, lane)) {
const interleaved = queue.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(queue);
} else {
update.next = interleaved.next;
interleaved.next = update;
}
queue.interleaved = update;
} else {
const pending = queue.pending;
if (pending === null) {
// This is the first update. Create a circular list.
update.next = update;
} else {
update.next = pending.next;
pending.next = update;
}
queue.pending = update;
}
enqueueUpdate(fiber, queue, update, lane);

const alternate = fiber.alternate;
if (
fiber.lanes === NoLanes &&
(alternate === null || alternate.lanes === NoLanes)
Expand Down Expand Up @@ -2377,28 +2272,101 @@ function dispatchSetState<S, A>(
warnIfNotCurrentlyActingUpdatesInDev(fiber);
}
}
const eventTime = requestEventTime();
const root = scheduleUpdateOnFiber(fiber, lane, eventTime);
if (root !== null) {
entangleTransitionUpdate(root, queue, lane);
}
}

markUpdateInDevTools(fiber, lane, action);
}

function isRenderPhaseUpdate(fiber: Fiber) {
const alternate = fiber.alternate;
return (
fiber === currentlyRenderingFiber ||
(alternate !== null && alternate === currentlyRenderingFiber)
);
}

function enqueueRenderPhaseUpdate<S, A>(
queue: UpdateQueue<S, A>,
update: Update<S, A>,
) {
// This is a render phase update. Stash it in a lazily-created map of
// queue -> linked list of updates. After this render pass, we'll restart
// and apply the stashed updates on top of the work-in-progress hook.
didScheduleRenderPhaseUpdateDuringThisPass = didScheduleRenderPhaseUpdate = true;
const pending = queue.pending;
if (pending === null) {
// This is the first update. Create a circular list.
update.next = update;
} else {
update.next = pending.next;
pending.next = update;
}
queue.pending = update;
}

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);
queue.lanes = newQueueLanes;
// Even if queue.lanes already include lane, we don't know for certain if
// the lane finished since the last time we entangled it. So we need to
// entangle it again, just to be sure.
markRootEntangled(root, newQueueLanes);
function enqueueUpdate<S, A>(
fiber: Fiber,
queue: UpdateQueue<S, A>,
update: Update<S, A>,
lane: Lane,
) {
if (isInterleavedUpdate(fiber, lane)) {
const interleaved = queue.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(queue);
} else {
update.next = interleaved.next;
interleaved.next = update;
}
queue.interleaved = update;
} else {
const pending = queue.pending;
if (pending === null) {
// This is the first update. Create a circular list.
update.next = update;
} else {
update.next = pending.next;
pending.next = update;
}
queue.pending = update;
}
}

function entangleTransitionUpdate<S, A>(
root: FiberRoot,
queue: UpdateQueue<S, A>,
lane: Lane,
) {
if (isTransitionLane(lane)) {
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);
queue.lanes = newQueueLanes;
// Even if queue.lanes already include lane, we don't know for certain if
// the lane finished since the last time we entangled it. So we need to
// entangle it again, just to be sure.
markRootEntangled(root, newQueueLanes);
}
}

function markUpdateInDevTools(fiber, lane, action) {
if (__DEV__) {
if (enableDebugTracing) {
if (fiber.mode & DebugTracingMode) {
Expand Down
Loading