Skip to content

Commit e44bd8d

Browse files
committed
Pass lanes as argument to performSyncWorkOnRoot
`performSyncWorkOnRoot` has only a single caller, and the caller already computes the next lanes (`getNextLanes`) before deciding to call the function. So we can pass them as an argument instead of computing the lanes again. There was already a TODO comment about this, but it was mostly perf related. However, @rickhanlonii noticed a discrepancy where the inner `getNextLanes` call was not being passed the current work-in- progress lanes. This is likely related to a regression found internally at Meta. We're still working on getting a proper regression test; I can come up with a contrived one but I'm not confident it'll be the same as the actual regression until we get a better repro. I suspect it might be related to the `enableUnifiedSyncLane` experiment.
1 parent fa43148 commit e44bd8d

File tree

2 files changed

+17
-17
lines changed

2 files changed

+17
-17
lines changed

packages/react-reconciler/src/ReactFiberRootScheduler.js

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -164,9 +164,6 @@ function flushSyncWorkAcrossRoots_impl(onlyLegacy: boolean) {
164164
return;
165165
}
166166

167-
const workInProgressRoot = getWorkInProgressRoot();
168-
const workInProgressRootRenderLanes = getWorkInProgressRootRenderLanes();
169-
170167
// There may or may not be synchronous work scheduled. Let's check.
171168
let didPerformSomeWork;
172169
let errors: Array<mixed> | null = null;
@@ -178,17 +175,18 @@ function flushSyncWorkAcrossRoots_impl(onlyLegacy: boolean) {
178175
if (onlyLegacy && root.tag !== LegacyRoot) {
179176
// Skip non-legacy roots.
180177
} else {
178+
const workInProgressRoot = getWorkInProgressRoot();
179+
const workInProgressRootRenderLanes =
180+
getWorkInProgressRootRenderLanes();
181181
const nextLanes = getNextLanes(
182182
root,
183183
root === workInProgressRoot ? workInProgressRootRenderLanes : NoLanes,
184184
);
185185
if (includesSyncLane(nextLanes)) {
186186
// This root has pending sync work. Flush it now.
187187
try {
188-
// TODO: Pass nextLanes as an argument instead of computing it again
189-
// inside performSyncWorkOnRoot.
190188
didPerformSomeWork = true;
191-
performSyncWorkOnRoot(root);
189+
performSyncWorkOnRoot(root, nextLanes);
192190
} catch (error) {
193191
// Collect errors so we can rethrow them at the end
194192
if (errors === null) {

packages/react-reconciler/src/ReactFiberWorkLoop.js

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1273,25 +1273,27 @@ function markRootSuspended(root: FiberRoot, suspendedLanes: Lanes) {
12731273

12741274
// This is the entry point for synchronous tasks that don't go
12751275
// through Scheduler
1276-
export function performSyncWorkOnRoot(root: FiberRoot): null {
1277-
if (enableProfilerTimer && enableProfilerNestedUpdatePhase) {
1278-
syncNestedUpdateFlag();
1279-
}
1280-
1276+
export function performSyncWorkOnRoot(root: FiberRoot, lanes: Lanes): null {
12811277
if ((executionContext & (RenderContext | CommitContext)) !== NoContext) {
12821278
throw new Error('Should not already be working.');
12831279
}
12841280

1285-
flushPassiveEffects();
1286-
1287-
// TODO: This was already computed in the caller. Pass it as an argument.
1288-
let lanes = getNextLanes(root, NoLanes);
1289-
if (!includesSyncLane(lanes)) {
1290-
// There's no remaining sync work left.
1281+
const didFlushPassiveEffects = flushPassiveEffects();
1282+
if (didFlushPassiveEffects) {
1283+
// If passive effects were flushed, exit to the outer work loop in the root
1284+
// scheduler, so we can recompute the priority.
1285+
// TODO: We don't actually need this `ensureRootIsScheduled` call because
1286+
// this path is only reachable if the root is already part of the schedule.
1287+
// I'm including it only for consistency with the other exit points from
1288+
// this function. Can address in a subsequent refactor.
12911289
ensureRootIsScheduled(root);
12921290
return null;
12931291
}
12941292

1293+
if (enableProfilerTimer && enableProfilerNestedUpdatePhase) {
1294+
syncNestedUpdateFlag();
1295+
}
1296+
12951297
let exitStatus = renderRootSync(root, lanes);
12961298
if (root.tag !== LegacyRoot && exitStatus === RootErrored) {
12971299
// If something threw an error, try rendering one more time. We'll render

0 commit comments

Comments
 (0)