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

Parallel transitions: Assign different lanes to consecutive transitions #20672

Merged
merged 2 commits into from
Feb 8, 2021
Merged

Parallel transitions: Assign different lanes to consecutive transitions #20672

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cb328f5
Land enableTransitionEntanglement changes
acdlite Feb 8, 2021
261cf29
Assign different lanes to consecutive transitions
acdlite Jan 27, 2021
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
Next Next commit
Land enableTransitionEntanglement changes 
Leaving the flag though because I plan to reuse it for additional,
similar changes.
  • Loading branch information
@acdlite
acdlite committed Feb 8, 2021
commit cb328f5b5e817948e23ad748165ab55da87d853d
72 changes: 16 additions & 56 deletions packages/react-reconciler/src/ReactFiberLane.new.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,7 @@ export type Lane = number;
export type LaneMap<T> = Array<T>;

import invariant from 'shared/invariant';
import {
enableCache,
enableTransitionEntanglement,
} from 'shared/ReactFeatureFlags';
import {enableCache} from 'shared/ReactFeatureFlags';

import {
ImmediatePriority as ImmediateSchedulerPriority,
Expand Down Expand Up @@ -309,15 +306,6 @@ export function getNextLanes(root: FiberRoot, wipLanes: Lanes): Lanes {
return NoLanes;
}

if (enableTransitionEntanglement) {
// We don't need to do anything extra here, because we apply per-lane
// transition entanglement in the entanglement loop below.
} else {
// If there are higher priority lanes, we'll include them even if they
// are suspended.
nextLanes = pendingLanes & getEqualOrHigherPriorityLanes(nextLanes);
}

// If we're already in the middle of a render, switching lanes will interrupt
// it and we'll lose our progress. We should only do this if the new lanes are
// higher priority.
Expand Down Expand Up @@ -595,16 +583,6 @@ function getHighestPriorityLane(lanes: Lanes) {
return lanes & -lanes;
}

function getLowestPriorityLane(lanes: Lanes): Lane {
// This finds the most significant non-zero bit.
const index = 31 - clz32(lanes);
return index < 0 ? NoLanes : 1 << index;
}

function getEqualOrHigherPriorityLanes(lanes: Lanes | Lane): Lanes {
return (getLowestPriorityLane(lanes) << 1) - 1;
}

export function pickArbitraryLane(lanes: Lanes): Lane {
// This wrapper function gets inlined. Only exists so to communicate that it
// doesn't matter which bit is selected; you can pick any bit without
Expand Down Expand Up @@ -676,39 +654,21 @@ export function markRootUpdated(
) {
root.pendingLanes |= updateLane;

// TODO: Theoretically, any update to any lane can unblock any other lane. But
// it's not practical to try every single possible combination. We need a
// heuristic to decide which lanes to attempt to render, and in which batches.
// For now, we use the same heuristic as in the old ExpirationTimes model:
// retry any lane at equal or lower priority, but don't try updates at higher
// priority without also including the lower priority updates. This works well
// when considering updates across different priority levels, but isn't
// sufficient for updates within the same priority, since we want to treat
// those updates as parallel.

// Unsuspend any update at equal or lower priority.
const higherPriorityLanes = updateLane - 1; // Turns 0b1000 into 0b0111

if (enableTransitionEntanglement) {
// If there are any suspended transitions, it's possible this new update
// could unblock them. Clear the suspended lanes so that we can try rendering
// them again.
//
// TODO: We really only need to unsuspend only lanes that are in the
// `subtreeLanes` of the updated fiber, or the update lanes of the return
// path. This would exclude suspended updates in an unrelated sibling tree,
// since there's no way for this update to unblock it.
//
// We don't do this if the incoming update is idle, because we never process
// idle updates until after all the regular updates have finished; there's no
// way it could unblock a transition.
if ((updateLane & IdleLanes) === NoLanes) {
root.suspendedLanes = NoLanes;
root.pingedLanes = NoLanes;
}
} else {
root.suspendedLanes &= higherPriorityLanes;
root.pingedLanes &= higherPriorityLanes;
// If there are any suspended transitions, it's possible this new update
// could unblock them. Clear the suspended lanes so that we can try rendering
// them again.
//
// TODO: We really only need to unsuspend only lanes that are in the
// `subtreeLanes` of the updated fiber, or the update lanes of the return
// path. This would exclude suspended updates in an unrelated sibling tree,
// since there's no way for this update to unblock it.
//
// We don't do this if the incoming update is idle, because we never process
// idle updates until after all the regular updates have finished; there's no
// way it could unblock a transition.
if ((updateLane & IdleLanes) === NoLanes) {
root.suspendedLanes = NoLanes;
root.pingedLanes = NoLanes;
}

const eventTimes = root.eventTimes;
Expand Down
72 changes: 16 additions & 56 deletions packages/react-reconciler/src/ReactFiberLane.old.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,7 @@ export type Lane = number;
export type LaneMap<T> = Array<T>;

import invariant from 'shared/invariant';
import {
enableCache,
enableTransitionEntanglement,
} from 'shared/ReactFeatureFlags';
import {enableCache} from 'shared/ReactFeatureFlags';

import {
ImmediatePriority as ImmediateSchedulerPriority,
Expand Down Expand Up @@ -309,15 +306,6 @@ export function getNextLanes(root: FiberRoot, wipLanes: Lanes): Lanes {
return NoLanes;
}

if (enableTransitionEntanglement) {
// We don't need to do anything extra here, because we apply per-lane
// transition entanglement in the entanglement loop below.
} else {
// If there are higher priority lanes, we'll include them even if they
// are suspended.
nextLanes = pendingLanes & getEqualOrHigherPriorityLanes(nextLanes);
}

// If we're already in the middle of a render, switching lanes will interrupt
// it and we'll lose our progress. We should only do this if the new lanes are
// higher priority.
Expand Down Expand Up @@ -595,16 +583,6 @@ function getHighestPriorityLane(lanes: Lanes) {
return lanes & -lanes;
}

function getLowestPriorityLane(lanes: Lanes): Lane {
// This finds the most significant non-zero bit.
const index = 31 - clz32(lanes);
return index < 0 ? NoLanes : 1 << index;
}

function getEqualOrHigherPriorityLanes(lanes: Lanes | Lane): Lanes {
return (getLowestPriorityLane(lanes) << 1) - 1;
}

export function pickArbitraryLane(lanes: Lanes): Lane {
// This wrapper function gets inlined. Only exists so to communicate that it
// doesn't matter which bit is selected; you can pick any bit without
Expand Down Expand Up @@ -676,39 +654,21 @@ export function markRootUpdated(
) {
root.pendingLanes |= updateLane;

// TODO: Theoretically, any update to any lane can unblock any other lane. But
// it's not practical to try every single possible combination. We need a
// heuristic to decide which lanes to attempt to render, and in which batches.
// For now, we use the same heuristic as in the old ExpirationTimes model:
// retry any lane at equal or lower priority, but don't try updates at higher
// priority without also including the lower priority updates. This works well
// when considering updates across different priority levels, but isn't
// sufficient for updates within the same priority, since we want to treat
// those updates as parallel.

// Unsuspend any update at equal or lower priority.
const higherPriorityLanes = updateLane - 1; // Turns 0b1000 into 0b0111

if (enableTransitionEntanglement) {
// If there are any suspended transitions, it's possible this new update
// could unblock them. Clear the suspended lanes so that we can try rendering
// them again.
//
// TODO: We really only need to unsuspend only lanes that are in the
// `subtreeLanes` of the updated fiber, or the update lanes of the return
// path. This would exclude suspended updates in an unrelated sibling tree,
// since there's no way for this update to unblock it.
//
// We don't do this if the incoming update is idle, because we never process
// idle updates until after all the regular updates have finished; there's no
// way it could unblock a transition.
if ((updateLane & IdleLanes) === NoLanes) {
root.suspendedLanes = NoLanes;
root.pingedLanes = NoLanes;
}
} else {
root.suspendedLanes &= higherPriorityLanes;
root.pingedLanes &= higherPriorityLanes;
// If there are any suspended transitions, it's possible this new update
// could unblock them. Clear the suspended lanes so that we can try rendering
// them again.
//
// TODO: We really only need to unsuspend only lanes that are in the
// `subtreeLanes` of the updated fiber, or the update lanes of the return
// path. This would exclude suspended updates in an unrelated sibling tree,
// since there's no way for this update to unblock it.
//
// We don't do this if the incoming update is idle, because we never process
// idle updates until after all the regular updates have finished; there's no
// way it could unblock a transition.
if ((updateLane & IdleLanes) === NoLanes) {
root.suspendedLanes = NoLanes;
root.pingedLanes = NoLanes;
}

const eventTimes = root.eventTimes;
Expand Down
17 changes: 4 additions & 13 deletions packages/react-reconciler/src/__tests__/ReactExpiration-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -733,19 +733,10 @@ describe('ReactExpiration', () => {
// Both normal pri updates should have expired.
expect(Scheduler).toFlushExpired([
'Sibling',
gate(flags => flags.enableTransitionEntanglement)
? // Notice that the high pri update didn't flush yet. Expiring one lane
// doesn't affect other lanes. (Unless they are intentionally
// entangled, like we do for overlapping transitions that affect the
// same state.)
'High pri: 0'
: // In the current implementation, once we pick the next lanes to work
// on, we entangle it with all pending at equal or higher priority.
// We could feasibly change this heuristic so that the high pri
// update doesn't render until after the expired updates have
// finished. But the important thing in this test is that the normal
// updates expired.
'High pri: 1',
// Notice that the high pri update didn't flush yet. Expiring one lane
// doesn't affect other lanes. (Unless they are intentionally entangled,
// like we do for overlapping transitions that affect the same state.)
'High pri: 0',
'Normal pri: 2',
'Sibling',
]);
Expand Down