Reuse work that was preempted if it was untouched
This tries to reuse work that was completed but another higher priority event came in. This tries to avoid starvation when high priority events causes low pri work to keep rerendering from scratch.
Sebastian Markbage committed
Jun 20, 2016 at 14:16 UTC
d8f785165c4a03d8d4836c2b5e47b1c15f3914b6
4 files changed
+112
-5
src/renderers/shared/fiber/ReactFiber.js
+9
@@ -82,6 +82,13 @@ export type Fiber = Instance & {
82
// memory if we need to.
83
alternate: ?Fiber,
84
85
+ // Keeps track if we've completed this node or if we're currently in the
86
+ // middle of processing it. We really should know this based on pendingProps
87
+ // or something else. We could also reuse the tag for this purpose. However,
88
+ // I'm not really sure so I'll use a flag for now.
89
+ // TODO: Find another way to infer this flag.
90
+ hasWorkInProgress: bool,
91
+
92
// Conceptual aliases
93
// parent : Instance -> returnFiber The parent happens to be the same as the return fiber.
94
// workInProgress : Fiber -> alternate The alternate used for reuse happens to be the same as work in progress.
@@ -118,6 +125,8 @@ var createFiber = function(tag : TypeOfWork, key : null | string) : Fiber {
125
126
alternate: null,
127
128
+ hasWorkInProgress: false,
129
+
130
};
131
};
132
src/renderers/shared/fiber/ReactFiberBeginWork.js
+16
-5
@@ -169,11 +169,22 @@ function beginWork(current : ?Fiber, workInProgress : Fiber) : ?Fiber {
169
return null;
170
}
171
}
172
- // if (workInProgress.pendingProps === workInProgress.memoizedProps) {
173
- // In a ping-pong scenario, this version could actually contain the
174
- // old props. In that case, we should be able to just bail out but I
175
- // don't yet quite understand what should happen on the reused node here.
176
- // }
172
+
173
+ if (!workInProgress.hasWorkInProgress &&
174
+ workInProgress.pendingProps === workInProgress.memoizedProps &&
175
+ workInProgress.pendingWorkPriority === NoWork) {
176
+ // If we started this work before, and finished it, or if we're in a
177
+ // ping-pong update scenario, this version could already be what we're
178
+ // looking for. In that case, we should be able to just bail out.
179
+ workInProgress.pendingProps = null;
180
+ // TODO: We should be able to bail out if there is remaining work at a lower
181
+ // priority too. However, I don't know if that is safe or even better since
182
+ // the other tree could've potentially finished that work.
183
+ return null;
184
+ }
185
+
186
+ workInProgress.hasWorkInProgress = true;
187
+
188
switch (workInProgress.tag) {
189
case IndeterminateComponent:
190
mountIndeterminateComponent(current, workInProgress);
src/renderers/shared/fiber/ReactFiberScheduler.js
+3
@@ -90,6 +90,9 @@ module.exports = function<T, P, I>(config : HostConfig<T, P, I>) {
90
// The work is now done. We don't need this anymore. This flags
91
// to the system not to redo any work here.
92
workInProgress.pendingProps = null;
93
+ if (workInProgress.pendingWorkPriority === NoWork) {
94
+ workInProgress.hasWorkInProgress = false;
95
+ }
96
97
// TODO: Stop using the parent for this purpose. I think this will break
98
// down in edge cases because when nodes are reused during bailouts, we
src/renderers/shared/fiber/__tests__/ReactIncremental-test.js
+84
@@ -353,4 +353,88 @@ describe('ReactIncremental', function() {
353
354
});
355
356
+ it('can reuse work done after being preempted', function() {
357
+
358
+ var ops = [];
359
+
360
+ function Bar(props) {
361
+ ops.push('Bar');
362
+ return <div>{props.children}</div>;
363
+ }
364
+
365
+ function Middle(props) {
366
+ ops.push('Middle');
367
+ return <span>{props.children}</span>;
368
+ }
369
+
370
+ var middleContent = (
371
+ <div>
372
+ <Middle>Hello</Middle>
373
+ <Bar>-</Bar>
374
+ <Middle>World</Middle>
375
+ </div>
376
+ );
377
+
378
+ function Foo(props) {
379
+ ops.push('Foo');
380
+ return (
381
+ <div>
382
+ <Bar>{props.text}</Bar>
383
+ <div hidden={true}>
384
+ {
385
+ props.step === 0 ?
386
+ <div>
387
+ <Middle>Hi</Middle>
388
+ <Bar>{props.text}</Bar>
389
+ <Middle>There</Middle>
390
+ </div>
391
+ : middleContent
392
+ }
393
+ </div>
394
+ </div>
395
+ );
396
+ }
397
+
398
+ // Start rendering an update
399
+
400
+ // Init
401
+ ReactNoop.render(<Foo text="foo" step={0} />);
402
+ ReactNoop.flush();
403
+
404
+ expect(ops).toEqual(['Foo', 'Bar', 'Middle', 'Bar', 'Middle']);
405
+
406
+ ops = [];
407
+
408
+ // Make a quick update which will schedule low priority work to
409
+ // update the middle content.
410
+ ReactNoop.render(<Foo text="bar" step={1} />);
411
+ ReactNoop.flushLowPri(30);
412
+
413
+ expect(ops).toEqual(['Foo', 'Bar']);
414
+
415
+ ops = [];
416
+
417
+ // The middle content is now pending rendering...
418
+ ReactNoop.flushLowPri(30);
419
+ expect(ops).toEqual(['Middle', 'Bar']);
420
+
421
+ ops = [];
422
+
423
+ // but we'll interupt it to render some higher priority work.
424
+ // The middle content will bailout so it remains untouched.
425
+ ReactNoop.render(<Foo text="foo" step={1} />);
426
+ ReactNoop.flushLowPri(30);
427
+
428
+ expect(ops).toEqual(['Foo', 'Bar']);
429
+
430
+ ops = [];
431
+
432
+ // Since we did nothing to the middle subtree during the interuption,
433
+ // we should be able to reuse the reconciliation work that we already did
434
+ // without restarting.
435
+ ReactNoop.flush();
436
+ expect(ops).toEqual(['Middle']);
437
+
438
+ });
439
+
440
});