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

Eager bailouts only for use state #18241

Closed
wants to merge 11 commits into from
Closed

Eager bailouts only for use state #18241

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
4257646
Inline implementation of useReducer into useState
Andarist Mar 6, 2020
8cec4df
Add failing test for dispatched actions being kept in the queue and p…
Andarist Mar 6, 2020
d1333c7
Remove eagerReducer bailout optimization from useReducer
Andarist Mar 6, 2020
50434b1
Remove constant condition
Andarist Mar 6, 2020
d75c389
Remove Update.eagerReducer in favor of Update.eagerlyComputed
Andarist Mar 6, 2020
3b5d4c5
Remove no longer needed UpdateQueue.lastRenderedReducer
Andarist Mar 6, 2020
c7843fc
Dont append updates to the queue on successful setState bailouts
Andarist Mar 6, 2020
97a67da
Fold back updateState into useReducer and rerenderState into rerender…
Andarist Mar 6, 2020
9e52006
fix Flow errors
Andarist Mar 6, 2020
2aaa9cf
update code comments
Andarist Mar 6, 2020
897945f
format files with prettier
Andarist Mar 6, 2020
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
159 changes: 104 additions & 55 deletions packages/react-reconciler/src/ReactFiberHooks.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ type Update<S, A> = {|
expirationTime: ExpirationTime,
suspenseConfig: null | SuspenseConfig,
action: A,
eagerReducer: ((S, A) => S) | null,
eagerlyComputed: boolean,
eagerState: S | null,
next: Update<S, A>,
priority?: ReactPriorityLevel,
Expand All @@ -112,7 +112,6 @@ type Update<S, A> = {|
type UpdateQueue<S, A> = {|
pending: Update<S, A> | null,
dispatch: (A => mixed) | null,
lastRenderedReducer: ((S, A) => S) | null,
lastRenderedState: S | null,
|};

Expand Down Expand Up @@ -641,7 +640,6 @@ function mountReducer<S, I, A>(
const queue = (hook.queue = {
pending: null,
dispatch: null,
lastRenderedReducer: reducer,
lastRenderedState: (initialState: any),
});
const dispatch: Dispatch<A> = (queue.dispatch = (dispatchAction.bind(
Expand All @@ -664,8 +662,6 @@ function updateReducer<S, I, A>(
'Should have a queue. This is likely a bug in React. Please file an issue.',
);

queue.lastRenderedReducer = reducer;

const current: Hook = (currentHook: any);

// The last rebase update that is NOT part of the base state.
Expand Down Expand Up @@ -706,7 +702,7 @@ function updateReducer<S, I, A>(
expirationTime: update.expirationTime,
suspenseConfig: update.suspenseConfig,
action: update.action,
eagerReducer: update.eagerReducer,
eagerlyComputed: update.eagerlyComputed,
eagerState: update.eagerState,
next: (null: any),
};
Expand All @@ -729,7 +725,7 @@ function updateReducer<S, I, A>(
expirationTime: Sync, // This update is going to be committed so we never want uncommit it.
suspenseConfig: update.suspenseConfig,
action: update.action,
eagerReducer: update.eagerReducer,
eagerlyComputed: update.eagerlyComputed,
eagerState: update.eagerState,
next: (null: any),
};
Expand All @@ -747,10 +743,9 @@ function updateReducer<S, I, A>(
update.suspenseConfig,
);

// Process this update.
if (update.eagerReducer === reducer) {
// If this update was processed eagerly, and its reducer matches the
// current reducer, we can use the eagerly computed state.
if (update.eagerlyComputed) {
// If this update was processed eagerly we can use the eagerly computed state.
// This can only happen for useState for which reducer is always the same.
newState = ((update.eagerState: any): S);
} else {
const action = update.action;
Expand Down Expand Up @@ -795,8 +790,6 @@ function rerenderReducer<S, I, A>(
'Should have a queue. This is likely a bug in React. Please file an issue.',
);

queue.lastRenderedReducer = reducer;

// This is a re-render. Apply the new render phase updates to the previous
// work-in-progress hook.
const dispatch: Dispatch<A> = (queue.dispatch: any);
Expand Down Expand Up @@ -838,7 +831,7 @@ function rerenderReducer<S, I, A>(
}

function mountState<S>(
initialState: (() => S) | S,
initialState: BasicStateAction<S>,
): [S, Dispatch<BasicStateAction<S>>] {
const hook = mountWorkInProgressHook();
if (typeof initialState === 'function') {
Expand All @@ -849,12 +842,11 @@ function mountState<S>(
const queue = (hook.queue = {
pending: null,
dispatch: null,
lastRenderedReducer: basicStateReducer,
lastRenderedState: (initialState: any),
});
const dispatch: Dispatch<
BasicStateAction<S>,
> = (queue.dispatch = (dispatchAction.bind(
> = (queue.dispatch = (setState.bind(
null,
currentlyRenderingFiber,
queue,
Expand All @@ -863,7 +855,7 @@ function mountState<S>(
}

function updateState<S>(
initialState: (() => S) | S,
initialState: BasicStateAction<S>,
): [S, Dispatch<BasicStateAction<S>>] {
return updateReducer(basicStateReducer, (initialState: any));
}
Expand Down Expand Up @@ -1260,6 +1252,18 @@ function rerenderTransition(
return [start, isPending];
}

function appendUpdate<S, A>(queue: UpdateQueue<S, A>, update: Update<S, A>) {
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 dispatchAction<S, A>(
fiber: Fiber,
queue: UpdateQueue<S, A>,
Expand All @@ -1268,7 +1272,7 @@ function dispatchAction<S, A>(
if (__DEV__) {
if (typeof arguments[3] === 'function') {
console.error(
"State updates from the useState() and useReducer() Hooks don't support the " +
"State updates from the useReducer() Hook don't support the " +
'second callback argument. To execute a side effect after ' +
'rendering, declare it in the component body with useEffect().',
);
Expand All @@ -1287,7 +1291,7 @@ function dispatchAction<S, A>(
expirationTime,
suspenseConfig,
action,
eagerReducer: null,
eagerlyComputed: false,
eagerState: null,
next: (null: any),
};
Expand All @@ -1296,22 +1300,73 @@ function dispatchAction<S, A>(
update.priority = getCurrentPriorityLevel();
}

// Append the update to the end of the list.
const pending = queue.pending;
if (pending === null) {
// This is the first update. Create a circular list.
update.next = update;
appendUpdate(queue, update);

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.
didScheduleRenderPhaseUpdate = true;
update.expirationTime = renderExpirationTime;
currentlyRenderingFiber.expirationTime = renderExpirationTime;
} else {
update.next = pending.next;
pending.next = update;
if (__DEV__) {
// $FlowExpectedError - jest isn't a global, and isn't recognized outside of tests
if ('undefined' !== typeof jest) {
warnIfNotScopedWithMatchingAct(fiber);
warnIfNotCurrentlyActingUpdatesInDev(fiber);
}
}
scheduleWork(fiber, expirationTime);
}
}

function setState<S>(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

For now this is being split from the dispatchAction but they both share pretty much the same implementation - besides the fact that dispatchAction has no eager bailout implemented.

Both could be collapsed into a single function and the eager bailout could only be attempted for reducer === basicStateReducer but to do that I would have to bring back keeping track of lastRenderedReducer.

Let me know which approach do you prefer.

fiber: Fiber,
queue: UpdateQueue<S, BasicStateAction<S>>,
action: BasicStateAction<S>,
) {
if (__DEV__) {
if (typeof arguments[3] === 'function') {
console.error(
"State updates from the useState() Hook don't support the " +
'second callback argument. To execute a side effect after ' +
'rendering, declare it in the component body with useEffect().',
);
}
}

const currentTime = requestCurrentTimeForUpdate();
const suspenseConfig = requestCurrentSuspenseConfig();
const expirationTime = computeExpirationForFiber(
currentTime,
fiber,
suspenseConfig,
);

const update: Update<S, BasicStateAction<S>> = {
expirationTime,
suspenseConfig,
action,
eagerlyComputed: false,
eagerState: null,
next: (null: any),
};

if (__DEV__) {
update.priority = getCurrentPriorityLevel();
}
queue.pending = update;

const alternate = fiber.alternate;
if (
fiber === currentlyRenderingFiber ||
(alternate !== null && alternate === currentlyRenderingFiber)
) {
appendUpdate(queue, update);
// 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.
Expand All @@ -1326,35 +1381,27 @@ function dispatchAction<S, A>(
// The queue is currently empty, which means we can eagerly compute the
// next state before entering the render phase. If the new state is the
// same as the current state, we may be able to bail out entirely.
const lastRenderedReducer = queue.lastRenderedReducer;
if (lastRenderedReducer !== null) {
let prevDispatcher;
if (__DEV__) {
prevDispatcher = ReactCurrentDispatcher.current;
ReactCurrentDispatcher.current = InvalidNestedHooksDispatcherOnUpdateInDEV;
let prevDispatcher;
if (__DEV__) {
prevDispatcher = ReactCurrentDispatcher.current;
ReactCurrentDispatcher.current = InvalidNestedHooksDispatcherOnUpdateInDEV;
}
try {
const currentState: S = (queue.lastRenderedState: any);
const eagerState = basicStateReducer(currentState, action);
if (is(eagerState, currentState)) {
// Fast path. We can bail out without scheduling React to re-render.
return;
}
try {
const currentState: S = (queue.lastRenderedState: any);
const eagerState = lastRenderedReducer(currentState, action);
// Stash the eagerly computed state, and the reducer used to compute
// it, on the update object. If the reducer hasn't changed by the
// time we enter the render phase, then the eager state can be used
// without calling the reducer again.
update.eagerReducer = lastRenderedReducer;
update.eagerState = eagerState;
if (is(eagerState, currentState)) {
// Fast path. We can bail out without scheduling React to re-render.
// It's still possible that we'll need to rebase this update later,
// if the component re-renders for a different reason and by that
// time the reducer has changed.
return;
}
} catch (error) {
// Suppress the error. It will throw again in the render phase.
} finally {
if (__DEV__) {
ReactCurrentDispatcher.current = prevDispatcher;
}
// Stash the eagerly computed state on the update object.
// It will be reused without without calling basicStateReducer again.
update.eagerlyComputed = true;
update.eagerState = eagerState;
} catch (error) {
// Suppress the error. It will throw again in the render phase.
} finally {
if (__DEV__) {
ReactCurrentDispatcher.current = prevDispatcher;
}
}
}
Expand All @@ -1365,6 +1412,8 @@ function dispatchAction<S, A>(
warnIfNotCurrentlyActingUpdatesInDev(fiber);
}
}

appendUpdate(queue, update);
scheduleWork(fiber, expirationTime);
}
}
Expand Down
4 changes: 2 additions & 2 deletions packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@ describe('ReactHooks', () => {
}),
);
}).toErrorDev(
'State updates from the useState() and useReducer() Hooks ' +
'State updates from the useState() Hook ' +
"don't support the second callback argument. " +
'To execute a side effect after rendering, ' +
'declare it in the component body with useEffect().',
Expand Down Expand Up @@ -304,7 +304,7 @@ describe('ReactHooks', () => {
}),
);
}).toErrorDev(
'State updates from the useState() and useReducer() Hooks ' +
'State updates from the useReducer() Hook ' +
"don't support the second callback argument. " +
'To execute a side effect after rendering, ' +
'declare it in the component body with useEffect().',
Expand Down
Loading