Skip to content

Commit 0d9802f

Browse files
acdliteacdlite
committed
When a root expires, flush all expired work in a single batch
Instead of flushing each level one at a time.
1 parent bb62722 commit 0d9802f

File tree

5 files changed

+240
-5
lines changed

5 files changed

+240
-5
lines changed

‎packages/react-noop-renderer/src/createReactNoop.js‎

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ if (__DEV__) {
4040

4141
function createReactNoop(reconciler: Function, useMutation: boolean) {
4242
let scheduledCallback = null;
43+
let scheduledCallbackTimeout = -1;
4344
let instanceCounter = 0;
4445
let hostDiffCounter = 0;
4546
let hostUpdateCounter = 0;
@@ -251,14 +252,27 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
251252
return inst;
252253
},
253254

254-
scheduleDeferredCallback(callback) {
255+
scheduleDeferredCallback(callback, options) {
255256
if (scheduledCallback) {
256257
throw new Error(
257258
'Scheduling a callback twice is excessive. Instead, keep track of ' +
258259
'whether the callback has already been scheduled.',
259260
);
260261
}
261262
scheduledCallback = callback;
263+
if (
264+
typeof options === 'object' &&
265+
options !== null &&
266+
typeof options.timeout === 'number'
267+
) {
268+
const newTimeout = options.timeout;
269+
if (
270+
scheduledCallbackTimeout === -1 ||
271+
scheduledCallbackTimeout > newTimeout
272+
) {
273+
scheduledCallbackTimeout = newTimeout;
274+
}
275+
}
262276
return 0;
263277
},
264278

@@ -267,6 +281,7 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
267281
throw new Error('No callback is scheduled.');
268282
}
269283
scheduledCallback = null;
284+
scheduledCallbackTimeout = -1;
270285
},
271286

272287
scheduleTimeout: setTimeout,
@@ -409,10 +424,9 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
409424
didStop = true;
410425
return 0;
411426
},
412-
// React's scheduler has its own way of keeping track of expired
413-
// work and doesn't read this, so don't bother setting it to the
414-
// correct value.
415-
didTimeout: false,
427+
didTimeout:
428+
scheduledCallbackTimeout !== -1 &&
429+
elapsedTimeInMs > scheduledCallbackTimeout,
416430
});
417431

418432
if (yieldedValues !== null) {

‎packages/react-reconciler/src/ReactFiberPendingPriority.js‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -242,6 +242,17 @@ export function findEarliestOutstandingPriorityLevel(
242242
return earliestExpirationTime;
243243
}
244244

245+
export function didExpireAtExpirationTime(
246+
root: FiberRoot,
247+
currentTime: ExpirationTime,
248+
): void {
249+
const expirationTime = root.expirationTime;
250+
if (expirationTime !== NoWork && currentTime >= expirationTime) {
251+
// The root has expired. Flush all work up to the current time.
252+
root.nextExpirationTimeToWorkOn = currentTime;
253+
}
254+
}
255+
245256
function findNextExpirationTimeToWorkOn(completedExpirationTime, root) {
246257
const earliestSuspendedTime = root.earliestSuspendedTime;
247258
const latestSuspendedTime = root.latestSuspendedTime;

‎packages/react-reconciler/src/ReactFiberScheduler.js‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ import {
8080
hasLowerPriorityWork,
8181
isPriorityLevelSuspended,
8282
findEarliestOutstandingPriorityLevel,
83+
didExpireAtExpirationTime,
8384
} from './ReactFiberPendingPriority';
8485
import {
8586
recordEffect,
@@ -2109,6 +2110,22 @@ function findHighestPriorityRoot() {
21092110
}
21102111

21112112
function performAsyncWork(dl) {
2113+
if (dl.didTimeout) {
2114+
// The callback timed out. That means at least one update has expired.
2115+
// Iterate through the root schedule. If they contain expired work, set
2116+
// the next render expiration time to the current time. This has the effect
2117+
// of flushing all expired work in a single batch, instead of flushing each
2118+
// level one at a time.
2119+
if (firstScheduledRoot !== null) {
2120+
recomputeCurrentRendererTime();
2121+
let root: FiberRoot = firstScheduledRoot;
2122+
do {
2123+
didExpireAtExpirationTime(root, currentRendererTime);
2124+
// The root schedule is circular, so this is never null.
2125+
root = (root.nextScheduledRoot: any);
2126+
} while (root !== firstScheduledRoot);
2127+
}
2128+
}
21122129
performWork(NoWork, dl);
21132130
}
21142131

‎packages/react-reconciler/src/__tests__/ReactIncrementalUpdates-test.internal.js‎

Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -453,4 +453,153 @@ describe('ReactIncrementalUpdates', () => {
453453
});
454454
expect(ReactNoop.getChildren()).toEqual([span('derived state')]);
455455
});
456+
457+
it('flushes all expired updates in a single batch', () => {
458+
class Foo extends React.Component {
459+
componentDidUpdate() {
460+
ReactNoop.yield('Commit: ' + this.props.prop);
461+
}
462+
componentDidMount() {
463+
ReactNoop.yield('Commit: ' + this.props.prop);
464+
}
465+
render() {
466+
ReactNoop.yield('Render: ' + this.props.prop);
467+
return <span prop={this.props.prop} />;
468+
}
469+
}
470+
471+
// First, as a sanity check, assert what happens when four low pri
472+
// updates in separate batches are all flushed in the same callback
473+
ReactNoop.render(<Foo prop="" />);
474+
ReactNoop.expire(1000);
475+
jest.advanceTimersByTime(1000);
476+
ReactNoop.render(<Foo prop="he" />);
477+
ReactNoop.expire(1000);
478+
jest.advanceTimersByTime(1000);
479+
ReactNoop.render(<Foo prop="hell" />);
480+
ReactNoop.expire(1000);
481+
jest.advanceTimersByTime(1000);
482+
ReactNoop.render(<Foo prop="hello" />);
483+
484+
// There should be a separate render and commit for each update
485+
expect(ReactNoop.flush()).toEqual([
486+
'Render: ',
487+
'Commit: ',
488+
'Render: he',
489+
'Commit: he',
490+
'Render: hell',
491+
'Commit: hell',
492+
'Render: hello',
493+
'Commit: hello',
494+
]);
495+
expect(ReactNoop.getChildren()).toEqual([span('hello')]);
496+
497+
// Now do the same thing, except this time expire all the updates
498+
// before flushing them.
499+
ReactNoop.render(<Foo prop="" />);
500+
ReactNoop.expire(1000);
501+
jest.advanceTimersByTime(1000);
502+
ReactNoop.render(<Foo prop="go" />);
503+
ReactNoop.expire(1000);
504+
jest.advanceTimersByTime(1000);
505+
ReactNoop.render(<Foo prop="good" />);
506+
ReactNoop.expire(1000);
507+
jest.advanceTimersByTime(1000);
508+
ReactNoop.render(<Foo prop="goodbye" />);
509+
510+
ReactNoop.advanceTime(10000);
511+
jest.advanceTimersByTime(10000);
512+
513+
// All the updates should render and commit in a single batch.
514+
expect(ReactNoop.flush()).toEqual(['Render: goodbye', 'Commit: goodbye']);
515+
expect(ReactNoop.getChildren()).toEqual([span('goodbye')]);
516+
});
517+
518+
it('flushes all expired updates in a single batch across multiple roots', () => {
519+
// Same as previous test, but with two roots.
520+
class Foo extends React.Component {
521+
componentDidUpdate() {
522+
ReactNoop.yield('Commit: ' + this.props.prop);
523+
}
524+
componentDidMount() {
525+
ReactNoop.yield('Commit: ' + this.props.prop);
526+
}
527+
render() {
528+
ReactNoop.yield('Render: ' + this.props.prop);
529+
return <span prop={this.props.prop} />;
530+
}
531+
}
532+
533+
// First, as a sanity check, assert what happens when four low pri
534+
// updates in separate batches are all flushed in the same callback
535+
ReactNoop.renderToRootWithID(<Foo prop="" />, 'a');
536+
ReactNoop.renderToRootWithID(<Foo prop="" />, 'b');
537+
538+
ReactNoop.expire(1000);
539+
jest.advanceTimersByTime(1000);
540+
ReactNoop.renderToRootWithID(<Foo prop="he" />, 'a');
541+
ReactNoop.renderToRootWithID(<Foo prop="he" />, 'b');
542+
543+
ReactNoop.expire(1000);
544+
jest.advanceTimersByTime(1000);
545+
ReactNoop.renderToRootWithID(<Foo prop="hell" />, 'a');
546+
ReactNoop.renderToRootWithID(<Foo prop="hell" />, 'b');
547+
548+
ReactNoop.expire(1000);
549+
jest.advanceTimersByTime(1000);
550+
ReactNoop.renderToRootWithID(<Foo prop="hello" />, 'a');
551+
ReactNoop.renderToRootWithID(<Foo prop="hello" />, 'b');
552+
553+
// There should be a separate render and commit for each update
554+
expect(ReactNoop.flush()).toEqual([
555+
'Render: ',
556+
'Commit: ',
557+
'Render: ',
558+
'Commit: ',
559+
'Render: he',
560+
'Commit: he',
561+
'Render: he',
562+
'Commit: he',
563+
'Render: hell',
564+
'Commit: hell',
565+
'Render: hell',
566+
'Commit: hell',
567+
'Render: hello',
568+
'Commit: hello',
569+
'Render: hello',
570+
'Commit: hello',
571+
]);
572+
expect(ReactNoop.getChildren('a')).toEqual([span('hello')]);
573+
expect(ReactNoop.getChildren('b')).toEqual([span('hello')]);
574+
575+
// Now do the same thing, except this time expire all the updates
576+
// before flushing them.
577+
ReactNoop.renderToRootWithID(<Foo prop="" />, 'a');
578+
ReactNoop.renderToRootWithID(<Foo prop="" />, 'b');
579+
ReactNoop.expire(1000);
580+
jest.advanceTimersByTime(1000);
581+
ReactNoop.renderToRootWithID(<Foo prop="go" />, 'a');
582+
ReactNoop.renderToRootWithID(<Foo prop="go" />, 'b');
583+
ReactNoop.expire(1000);
584+
jest.advanceTimersByTime(1000);
585+
ReactNoop.renderToRootWithID(<Foo prop="good" />, 'a');
586+
ReactNoop.renderToRootWithID(<Foo prop="good" />, 'b');
587+
ReactNoop.expire(1000);
588+
jest.advanceTimersByTime(1000);
589+
ReactNoop.renderToRootWithID(<Foo prop="goodbye" />, 'a');
590+
ReactNoop.renderToRootWithID(<Foo prop="goodbye" />, 'b');
591+
592+
ReactNoop.advanceTime(10000);
593+
jest.advanceTimersByTime(10000);
594+
595+
// All the updates should render and commit in a single batch.
596+
expect(ReactNoop.flush()).toEqual([
597+
'Render: goodbye',
598+
'Commit: goodbye',
599+
'Render: goodbye',
600+
'Commit: goodbye',
601+
]);
602+
expect(ReactNoop.getChildren('a')).toEqual([span('goodbye')]);
603+
expect(ReactNoop.getChildren('b')).toEqual([span('goodbye')]);
604+
});
456605
});

‎packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -782,6 +782,50 @@ describe('ReactSuspense', () => {
782782
expect(ReactNoop.getChildren()).toEqual([div(span('Async'))]);
783783
});
784784

785+
it('flushes all expired updates in a single batch', async () => {
786+
class Foo extends React.Component {
787+
componentDidUpdate() {
788+
ReactNoop.yield('Commit: ' + this.props.text);
789+
}
790+
componentDidMount() {
791+
ReactNoop.yield('Commit: ' + this.props.text);
792+
}
793+
render() {
794+
return (
795+
<Placeholder fallback={<Text text="Loading..." />}>
796+
<AsyncText ms={20000} text={this.props.text} />
797+
</Placeholder>
798+
);
799+
}
800+
}
801+
802+
ReactNoop.render(<Foo text="" />);
803+
ReactNoop.expire(1000);
804+
jest.advanceTimersByTime(1000);
805+
ReactNoop.render(<Foo text="go" />);
806+
ReactNoop.expire(1000);
807+
jest.advanceTimersByTime(1000);
808+
ReactNoop.render(<Foo text="good" />);
809+
ReactNoop.expire(1000);
810+
jest.advanceTimersByTime(1000);
811+
ReactNoop.render(<Foo text="goodbye" />);
812+
813+
ReactNoop.advanceTime(10000);
814+
jest.advanceTimersByTime(10000);
815+
816+
expect(ReactNoop.flush()).toEqual([
817+
'Suspend! [goodbye]',
818+
'Loading...',
819+
'Commit: goodbye',
820+
]);
821+
expect(ReactNoop.getChildren()).toEqual([span('Loading...')]);
822+
823+
ReactNoop.advanceTime(20000);
824+
await advanceTimers(20000);
825+
expect(ReactNoop.clearYields()).toEqual(['Promise resolved [goodbye]']);
826+
expect(ReactNoop.getChildren()).toEqual([span('Loading...')]);
827+
});
828+
785829
describe('a Delay component', () => {
786830
function Never() {
787831
// Throws a promise that resolves after some arbitrarily large

0 commit comments

Comments
 (0)