Skip to content

Commit

Permalink
Decouple expiration times and transition timeouts
Browse files Browse the repository at this point in the history
We currently use the expiration time to represent the timeout of a
transition. Since we intend to stop treating work priority as a
timeline, we can no longer use this trick.

In this commit, I've changed it to store the event time on the update
object instead. Long term, we will store event time on the root as a map
of transition -> event time. I'm only storing it on the update object
as a temporary workaround to unblock the rest of the changes.
  • Loading branch information
acdlite committed Dec 19, 2019
1 parent da8c9f2 commit e062ad7
Show file tree
Hide file tree
Showing 12 changed files with 107 additions and 160 deletions.
6 changes: 3 additions & 3 deletions packages/react-reconciler/src/ReactFiberClassComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ const classComponentUpdater = {
suspenseConfig,
);

const update = createUpdate(expirationTime, suspenseConfig);
const update = createUpdate(currentTime, expirationTime, suspenseConfig);
update.payload = payload;
if (callback !== undefined && callback !== null) {
if (__DEV__) {
Expand All @@ -212,7 +212,7 @@ const classComponentUpdater = {
suspenseConfig,
);

const update = createUpdate(expirationTime, suspenseConfig);
const update = createUpdate(currentTime, expirationTime, suspenseConfig);
update.tag = ReplaceState;
update.payload = payload;

Expand All @@ -236,7 +236,7 @@ const classComponentUpdater = {
suspenseConfig,
);

const update = createUpdate(expirationTime, suspenseConfig);
const update = createUpdate(currentTime, expirationTime, suspenseConfig);
update.tag = ForceUpdate;

if (callback !== undefined && callback !== null) {
Expand Down
11 changes: 4 additions & 7 deletions packages/react-reconciler/src/ReactFiberExpirationTime.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,16 +85,13 @@ export function computeAsyncExpiration(
);
}

export function computeSuspenseExpiration(
export function computeSuspenseTimeout(
currentTime: ExpirationTime,
timeoutMs: number,
): ExpirationTime {
// TODO: Should we warn if timeoutMs is lower than the normal pri expiration time?
return computeExpirationBucket(
currentTime,
timeoutMs,
LOW_PRIORITY_BATCH_SIZE,
);
const currentTimeMs = expirationTimeToMs(currentTime);
const deadlineMs = currentTimeMs + timeoutMs;
return msToExpirationTime(deadlineMs);
}

// We intentionally set a higher expiration time for interactive updates in
Expand Down
9 changes: 8 additions & 1 deletion packages/react-reconciler/src/ReactFiberHooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,9 @@ export type Dispatcher = {
};

type Update<S, A> = {
// TODO: Temporary field. Will remove this by storing a map of
// transition -> start time on the root.
eventTime: ExpirationTime,
expirationTime: ExpirationTime,
suspenseConfig: null | SuspenseConfig,
action: A,
Expand Down Expand Up @@ -717,6 +720,7 @@ function updateReducer<S, I, A>(
// skipped update, the previous update/state is the new base
// update/state.
const clone: Update<S, A> = {
eventTime: update.eventTime,
expirationTime: update.expirationTime,
suspenseConfig: update.suspenseConfig,
action: update.action,
Expand Down Expand Up @@ -748,8 +752,10 @@ function updateReducer<S, I, A>(
} else {
// This update does have sufficient priority.

const eventTime = update.eventTime;
if (newBaseQueueLast !== null) {
const clone: Update<S, A> = {
eventTime,
expirationTime: Sync, // This update is going to be committed so we never want uncommit it.
suspenseConfig: update.suspenseConfig,
action: update.action,
Expand All @@ -766,7 +772,7 @@ function updateReducer<S, I, A>(
// TODO: We should skip this update if it was already committed but currently
// we have no way of detecting the difference between a committed and suspended
// update here.
markRenderEventTimeAndConfig(updateExpirationTime, suspenseConfig);
markRenderEventTimeAndConfig(eventTime, suspenseConfig);

// Process this update.
if (update.eagerReducer === reducer) {
Expand Down Expand Up @@ -1404,6 +1410,7 @@ function dispatchAction<S, A>(
);

const update: Update<S, A> = {
eventTime: currentTime,
expirationTime,
suspenseConfig,
action,
Expand Down
2 changes: 1 addition & 1 deletion packages/react-reconciler/src/ReactFiberNewContext.js
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ export function propagateContextChange(

if (fiber.tag === ClassComponent) {
// Schedule a force update on the work-in-progress.
const update = createUpdate(renderExpirationTime, null);
const update = createUpdate(NoWork, renderExpirationTime, null);
update.tag = ForceUpdate;
// TODO: Because we don't have a work-in-progress, this will add the
// update to the current fiber, too, which means it will persist even if
Expand Down
2 changes: 1 addition & 1 deletion packages/react-reconciler/src/ReactFiberReconciler.js
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ export function updateContainer(
}
}

const update = createUpdate(expirationTime, suspenseConfig);
const update = createUpdate(currentTime, expirationTime, suspenseConfig);
// Caution: React DevTools currently depends on this property
// being called "element".
update.payload = {element};
Expand Down
8 changes: 4 additions & 4 deletions packages/react-reconciler/src/ReactFiberThrow.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ import {
checkForWrongSuspensePriorityInDEV,
} from './ReactFiberWorkLoop';

import {Sync} from './ReactFiberExpirationTime';
import {Sync, NoWork} from './ReactFiberExpirationTime';

const PossiblyWeakMap = typeof WeakMap === 'function' ? WeakMap : Map;

Expand All @@ -72,7 +72,7 @@ function createRootErrorUpdate(
errorInfo: CapturedValue<mixed>,
expirationTime: ExpirationTime,
): Update<mixed> {
const update = createUpdate(expirationTime, null);
const update = createUpdate(NoWork, expirationTime, null);
// Unmount the root by rendering null.
update.tag = CaptureUpdate;
// Caution: React DevTools currently depends on this property
Expand All @@ -91,7 +91,7 @@ function createClassErrorUpdate(
errorInfo: CapturedValue<mixed>,
expirationTime: ExpirationTime,
): Update<mixed> {
const update = createUpdate(expirationTime, null);
const update = createUpdate(NoWork, expirationTime, null);
update.tag = CaptureUpdate;
const getDerivedStateFromError = fiber.type.getDerivedStateFromError;
if (typeof getDerivedStateFromError === 'function') {
Expand Down Expand Up @@ -267,7 +267,7 @@ function throwException(
// When we try rendering again, we should not reuse the current fiber,
// since it's known to be in an inconsistent state. Use a force update to
// prevent a bail out.
const update = createUpdate(Sync, null);
const update = createUpdate(NoWork, Sync, null);
update.tag = ForceUpdate;
enqueueUpdate(sourceFiber, update);
}
Expand Down
35 changes: 5 additions & 30 deletions packages/react-reconciler/src/ReactFiberTransition.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,17 +53,18 @@ export function startTransition(
) {
const fiber = transitionInstance.fiber;

let resolvedConfig: SuspenseConfig | null =
const resolvedConfig: SuspenseConfig | null =
config === undefined ? null : config;

const currentTime = requestCurrentTimeForUpdate();

// TODO: runWithPriority shouldn't be necessary here. React should manage its
// own concept of priority, and only consult Scheduler for updates that are
// scheduled from outside a React context.
const priorityLevel = getCurrentPriorityLevel();
runWithPriority(
priorityLevel < UserBlockingPriority ? UserBlockingPriority : priorityLevel,
() => {
const currentTime = requestCurrentTimeForUpdate();
userBlockingExpirationTime = computeExpirationForFiber(
currentTime,
fiber,
Expand All @@ -75,7 +76,6 @@ export function startTransition(
runWithPriority(
priorityLevel > NormalPriority ? NormalPriority : priorityLevel,
() => {
const currentTime = requestCurrentTimeForUpdate();
let expirationTime = computeExpirationForFiber(
currentTime,
fiber,
Expand All @@ -85,34 +85,9 @@ export function startTransition(
// Because there's only a single transition per useTransition hook, we
// don't need a queue here; we can cheat by only tracking the most
// recently scheduled transition.
// TODO: This trick depends on transition expiration times being
// monotonically decreasing in priority, but since expiration times
// currently correspond to `timeoutMs`, that might not be true if
// `timeoutMs` changes to something smaller before the previous transition
// resolves. But this is a temporary edge case, since we're about to
// remove the correspondence between `timeoutMs` and the expiration time.
const oldPendingExpirationTime = transitionInstance.pendingExpirationTime;
while (
oldPendingExpirationTime !== NoWork &&
oldPendingExpirationTime <= expirationTime
) {
// Temporary hack to make pendingExpirationTime monotonically decreasing
if (resolvedConfig === null) {
resolvedConfig = {
timeoutMs: 5250,
};
} else {
resolvedConfig = {
timeoutMs: (resolvedConfig.timeoutMs | 0 || 5000) + 250,
busyDelayMs: resolvedConfig.busyDelayMs,
busyMinDurationMs: resolvedConfig.busyMinDurationMs,
};
}
expirationTime = computeExpirationForFiber(
currentTime,
fiber,
resolvedConfig,
);
if (oldPendingExpirationTime === expirationTime) {
expirationTime -= 1;
}
transitionInstance.pendingExpirationTime = expirationTime;

Expand Down
Loading

0 comments on commit e062ad7

Please sign in to comment.