Skip to content

Commit 97845e6

Browse files
committed
Pass lanes as argument to performSyncWorkOnRoot (#26763)
`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. Usually this shouldn't matter because there should never be work-in-progress sync work; it should finish immediately. There is one case I'm aware of where we exit the work loop without finishing a sync render, which is selective hydration, but even then it should switch to the sync hydration lane, not the normal sync lane. So something else is probably going on. I suspect it might be related to the `enableUnifiedSyncLane` experiment. 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. DiffTrain build for commit 13d0225.
1 parent 0c6a61b commit 97845e6

File tree

13 files changed

+950
-932
lines changed

13 files changed

+950
-932
lines changed

compiled-rn/facebook-fbsource/xplat/js/RKJSModules/vendor/react-test-renderer/cjs/ReactTestRenderer-dev.js

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* @noflow
88
* @nolint
99
* @preventMunge
10-
* @generated SignedSource<<0e3d907c9303927e282697c3cd932853>>
10+
* @generated SignedSource<<aab7866dd4ea4a33aa518692d1dfa02f>>
1111
*/
1212

1313
'use strict';
@@ -6078,10 +6078,7 @@ function flushSyncWorkAcrossRoots_impl(onlyLegacy) {
60786078
if (!mightHavePendingSyncWork) {
60796079
// Fast path. There's no sync work to do.
60806080
return;
6081-
}
6082-
6083-
var workInProgressRoot = getWorkInProgressRoot();
6084-
var workInProgressRootRenderLanes = getWorkInProgressRootRenderLanes(); // There may or may not be synchronous work scheduled. Let's check.
6081+
} // There may or may not be synchronous work scheduled. Let's check.
60856082

60866083
var didPerformSomeWork;
60876084
var errors = null;
@@ -6094,6 +6091,8 @@ function flushSyncWorkAcrossRoots_impl(onlyLegacy) {
60946091
while (root !== null) {
60956092
if (onlyLegacy && root.tag !== LegacyRoot);
60966093
else {
6094+
var workInProgressRoot = getWorkInProgressRoot();
6095+
var workInProgressRootRenderLanes = getWorkInProgressRootRenderLanes();
60976096
var nextLanes = getNextLanes(
60986097
root,
60996098
root === workInProgressRoot ? workInProgressRootRenderLanes : NoLanes
@@ -6102,10 +6101,8 @@ function flushSyncWorkAcrossRoots_impl(onlyLegacy) {
61026101
if (includesSyncLane(nextLanes)) {
61036102
// This root has pending sync work. Flush it now.
61046103
try {
6105-
// TODO: Pass nextLanes as an argument instead of computing it again
6106-
// inside performSyncWorkOnRoot.
61076104
didPerformSomeWork = true;
6108-
performSyncWorkOnRoot(root);
6105+
performSyncWorkOnRoot(root, nextLanes);
61096106
} catch (error) {
61106107
// Collect errors so we can rethrow them at the end
61116108
if (errors === null) {
@@ -20677,25 +20674,28 @@ function markRootSuspended(root, suspendedLanes) {
2067720674
} // This is the entry point for synchronous tasks that don't go
2067820675
// through Scheduler
2067920676

20680-
function performSyncWorkOnRoot(root) {
20681-
{
20682-
syncNestedUpdateFlag();
20683-
}
20684-
20677+
function performSyncWorkOnRoot(root, lanes) {
2068520678
if ((executionContext & (RenderContext | CommitContext)) !== NoContext) {
2068620679
throw new Error("Should not already be working.");
2068720680
}
2068820681

20689-
flushPassiveEffects(); // TODO: This was already computed in the caller. Pass it as an argument.
20690-
20691-
var lanes = getNextLanes(root, NoLanes);
20682+
var didFlushPassiveEffects = flushPassiveEffects();
2069220683

20693-
if (!includesSyncLane(lanes)) {
20694-
// There's no remaining sync work left.
20684+
if (didFlushPassiveEffects) {
20685+
// If passive effects were flushed, exit to the outer work loop in the root
20686+
// scheduler, so we can recompute the priority.
20687+
// TODO: We don't actually need this `ensureRootIsScheduled` call because
20688+
// this path is only reachable if the root is already part of the schedule.
20689+
// I'm including it only for consistency with the other exit points from
20690+
// this function. Can address in a subsequent refactor.
2069520691
ensureRootIsScheduled(root);
2069620692
return null;
2069720693
}
2069820694

20695+
{
20696+
syncNestedUpdateFlag();
20697+
}
20698+
2069920699
var exitStatus = renderRootSync(root, lanes);
2070020700

2070120701
if (root.tag !== LegacyRoot && exitStatus === RootErrored) {
@@ -23985,7 +23985,7 @@ function createFiberRoot(
2398523985
return root;
2398623986
}
2398723987

23988-
var ReactVersion = "18.3.0-canary-62512bafc-20230928";
23988+
var ReactVersion = "18.3.0-canary-13d0225c7-20230928";
2398923989

2399023990
// Might add PROFILE later.
2399123991

0 commit comments

Comments
 (0)