@samitouri / QOS-React / commits / f9af9aacd3

Down-prioritize children of `hidden` host components

To make sure we don't reset the priority of a down-prioritized fiber, we compare the priority we're currently rendering at to the fiber's work priority. If the work priority is lower, then we know not to reset the work priority.

Andrew Clark committed Jun 30, 2017 at 14:45 UTC f9af9aacd3c14288cd5869ff23cc71cdfb54d447

9 files changed +171 -121

scripts/fiber/tests-passing.txt

@@ -2030,6 +2030,8 @@ src/renderers/shared/fiber/__tests__/ReactIncremental-test.js

2030 * updates a previous render

2031 * can cancel partially rendered work and restart

2032 * should call callbacks even if updates are aborted

2033 +* can deprioritize unfinished work and resume it later

2034 +* can deprioritize a tree from without dropping work

2035 * memoizes work even if shouldComponentUpdate returns false

2036 * can update in the middle of a tree using setState

2037 * can queue multiple state updates

@@ -2042,6 +2044,7 @@ src/renderers/shared/fiber/__tests__/ReactIncremental-test.js

2044 * can handle if setState callback throws

2045 * merges and masks context

2046 * does not leak own context into context provider

2047 +* provides context when reusing work

2048 * reads context when setState is below the provider

2049 * reads context when setState is above the provider

2050 * maintains the correct context when providers bail out due to low priority

@@ -2085,6 +2088,7 @@ src/renderers/shared/fiber/__tests__/ReactIncrementalPerf-test.js

2088 * warns on cascading renders from top-level render

2089 * does not treat setState from cWM or cWRP as cascading

2090 * captures all lifecycles

2091 +* measures deprioritized work

2092 * measures deferred work in chunks

2093 * recovers from fatal errors

2094 * recovers from caught errors

@@ -2124,6 +2128,7 @@ src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js

2128 * can reuse side-effects after being preempted, if shouldComponentUpdate is false

2129 * can update a completed tree before it has a chance to commit

2130 * updates a child even though the old props is empty

2131 +* deprioritizes setStates that happens within a deprioritized tree

2132 * calls callback after update is flushed

2133 * calls setState callback even if component bails out

2134 * calls componentWillUnmount after a deletion, even if nested

src/renderers/shared/fiber/ReactChildFiber.js

+2 -2

@@ -1463,7 +1463,7 @@ exports.cloneChildFibers = function(

1463 let currentChild = workInProgress.child;

1464 let newChild = createWorkInProgress(currentChild, renderPriority);

1465 // TODO: Pass this as an argument, since it's easy to forget.

1466 - newChild.pendingProps = null;

1466 + newChild.pendingProps = currentChild.pendingProps;

1467 workInProgress.child = newChild;

1468

1469 newChild.return = workInProgress;

@@ -1473,7 +1473,7 @@ exports.cloneChildFibers = function(

1473 currentChild,

1474 renderPriority,

1475 );

1476 - newChild.pendingProps = null;

1476 + newChild.pendingProps = currentChild.pendingProps;

1477 newChild.return = workInProgress;

1478 }

1479 newChild.sibling = null;

src/renderers/shared/fiber/ReactFiber.js

@@ -451,3 +451,10 @@ exports.createFiberFromPortal = function(

451 };

452 return fiber;

453 };

454 +

455 +exports.largerPriority = function(

456 + p1: PriorityLevel,

457 + p2: PriorityLevel,

458 +): PriorityLevel {

459 + return p1 !== NoWork && (p2 === NoWork || p2 > p1) ? p1 : p2;

460 +};

src/renderers/shared/fiber/ReactFiberBeginWork.js

+38 -18

@@ -49,7 +49,7 @@ var {

49 YieldComponent,

50 Fragment,

51 } = ReactTypeOfWork;

52 -var {NoWork} = require('ReactPriorityLevel');

52 +var {NoWork, OffscreenPriority} = require('ReactPriorityLevel');

53 var {

54 PerformedWork,

55 Placement,

@@ -76,7 +76,11 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(

76 scheduleUpdate: (fiber: Fiber, priorityLevel: PriorityLevel) => void,

77 getPriorityContext: (fiber: Fiber, forceAsync: boolean) => PriorityLevel,

78 ) {

79 - const {shouldSetTextContent} = config;

79 + const {

80 + shouldSetTextContent,

81 + useSyncScheduling,

82 + shouldDeprioritizeSubtree,

83 + } = config;

85 const {pushHostContext, pushHostContainer} = hostContext;

@@ -375,28 +379,29 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(

379 return bailoutOnAlreadyFinishedWork(current, workInProgress);

380 }

381

378 - function updateHostComponent(current, workInProgress) {

382 + function updateHostComponent(current, workInProgress, renderPriority) {

383 pushHostContext(workInProgress);

384

385 if (current === null) {

386 tryToClaimNextHydratableInstance(workInProgress);

387 }

388

385 - let nextProps = workInProgress.pendingProps;

389 const type = workInProgress.type;

387 - const prevProps = current !== null ? current.memoizedProps : null;

390 const memoizedProps = workInProgress.memoizedProps;

391 + let nextProps = workInProgress.pendingProps;

392 + if (nextProps === null) {

393 + nextProps = memoizedProps;

394 + invariant(

395 + nextProps !== null,

396 + 'We should always have pending or current props. This error is ' +

397 + 'likely caused by a bug in React. Please file an issue.',

398 + );

399 + }

400 + const prevProps = current !== null ? current.memoizedProps : null;

401 +

402 if (hasContextChanged()) {

403 // Normally we can bail out on props equality but if context has changed

404 // we don't do the bailout and we have to reuse existing props instead.

392 - if (nextProps === null) {

393 - nextProps = memoizedProps;

394 - invariant(

395 - nextProps !== null,

396 - 'We should always have pending or current props. This error is ' +

397 - 'likely caused by a bug in React. Please file an issue.',

398 - );

399 - }

405 } else if (nextProps === null || memoizedProps === nextProps) {

406 return bailoutOnAlreadyFinishedWork(current, workInProgress);

407 }

@@ -418,6 +423,18 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(

423

424 markRef(current, workInProgress);

425

426 + // Check the host config to see if the children are offscreen/hidden.

427 + if (

428 + renderPriority !== OffscreenPriority &&

429 + !useSyncScheduling &&

430 + shouldDeprioritizeSubtree(type, nextProps)

431 + ) {

432 + // Down-prioritize the children.

433 + workInProgress.pendingWorkPriority = OffscreenPriority;

434 + // Bailout and come back to this fiber later at OffscreenPriority.

435 + return null;

436 + }

437 +

438 reconcileChildren(current, workInProgress, nextChildren);

439 memoizeProps(workInProgress, nextProps);

440 return workInProgress.child;

@@ -686,10 +703,9 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(

703 return null;

704 }

705

706 + // TODO: Delete memoizeProps/State and move to reconcile/bailout instead

707 function memoizeProps(workInProgress: Fiber, nextProps: any) {

708 workInProgress.memoizedProps = nextProps;

691 - // Reset the pending props

692 - workInProgress.pendingProps = null;

709 }

710

711 function memoizeState(workInProgress: Fiber, nextState: any) {

@@ -728,7 +744,7 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(

744 case HostRoot:

745 return updateHostRoot(current, workInProgress, priorityLevel);

746 case HostComponent:

731 - return updateHostComponent(current, workInProgress);

747 + return updateHostComponent(current, workInProgress, priorityLevel);

748 case HostText:

749 return updateHostText(current, workInProgress);

750 case CoroutineHandlerPhase:

@@ -793,13 +809,17 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(

809

810 // Unmount the current children as if the component rendered null

811 const nextChildren = null;

796 - reconcileChildren(current, workInProgress, nextChildren);

812 + reconcileChildrenAtPriority(

813 + current,

814 + workInProgress,

815 + nextChildren,

816 + priorityLevel,

817 + );

818

819 if (workInProgress.tag === ClassComponent) {

820 const instance = workInProgress.stateNode;

821 workInProgress.memoizedProps = instance.props;

822 workInProgress.memoizedState = instance.state;

802 - workInProgress.pendingProps = null;

823 }

824

825 return workInProgress.child;

src/renderers/shared/fiber/ReactFiberCompleteWork.js

+17 -2

@@ -14,6 +14,7 @@

15 import type {ReactCoroutine} from 'ReactTypes';

16 import type {Fiber} from 'ReactFiber';

17 +import type {PriorityLevel} from 'ReactPriorityLevel';

18 import type {HostContext} from 'ReactFiberHostContext';

19 import type {HydrationContext} from 'ReactFiberHydrationContext';

20 import type {FiberRoot} from 'ReactFiberRoot';

@@ -23,6 +24,7 @@ var {reconcileChildFibers} = require('ReactChildFiber');

24 var {popContextProvider} = require('ReactFiberContext');

25 var ReactTypeOfWork = require('ReactTypeOfWork');

26 var ReactTypeOfSideEffect = require('ReactTypeOfSideEffect');

27 +var ReactPriorityLevel = require('ReactPriorityLevel');

28 var {

29 IndeterminateComponent,

30 FunctionalComponent,

@@ -37,6 +39,7 @@ var {

39 Fragment,

40 } = ReactTypeOfWork;

41 var {Placement, Ref, Update} = ReactTypeOfSideEffect;

42 +var {OffscreenPriority} = ReactPriorityLevel;

44 if (__DEV__) {

45 var ReactDebugCurrentFiber = require('ReactDebugCurrentFiber');

@@ -181,11 +184,24 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(

184 function completeWork(

185 current: Fiber | null,

186 workInProgress: Fiber,

187 + renderPriority: PriorityLevel,

188 ): Fiber | null {

189 if (__DEV__) {

190 ReactDebugCurrentFiber.current = workInProgress;

191 }

192

193 + // Get the latest props.

194 + let newProps = workInProgress.pendingProps;

195 + if (newProps === null) {

196 + newProps = workInProgress.memoizedProps;

197 + } else if (

198 + workInProgress.pendingWorkPriority !== OffscreenPriority ||

199 + renderPriority === OffscreenPriority

200 + ) {

201 + // Reset the pending props, unless this was a down-prioritization.

202 + workInProgress.pendingProps = null;

203 + }

204 +

205 switch (workInProgress.tag) {

206 case FunctionalComponent:

207 return null;

@@ -216,7 +232,6 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(

232 popHostContext(workInProgress);

233 const rootContainerInstance = getRootHostContainer();

234 const type = workInProgress.type;

219 - const newProps = workInProgress.memoizedProps;

235 if (current !== null && workInProgress.stateNode != null) {

236 // If we have an alternate, that means this is an update and we need to

237 // schedule a side-effect to do the updates.

@@ -311,7 +326,7 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(

326 return null;

327 }

328 case HostText: {

314 - let newText = workInProgress.memoizedProps;

329 + let newText = newProps;

330 if (current && workInProgress.stateNode != null) {

331 const oldText = current.memoizedProps;

332 // If we have an alternate, that means this is an update and we need

src/renderers/shared/fiber/ReactFiberScheduler.js

+16 -10

@@ -48,7 +48,7 @@ var ReactFiberHydrationContext = require('ReactFiberHydrationContext');

48 var {ReactCurrentOwner} = require('ReactGlobalSharedState');

49 var getComponentName = require('getComponentName');

51 -var {createWorkInProgress} = require('ReactFiber');

51 +var {createWorkInProgress, largerPriority} = require('ReactFiber');

52 var {onCommitRoot} = require('ReactFiberDevToolsHook');

54 var {

@@ -551,7 +551,18 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(

551 priorityContext = previousPriorityContext;

552 }

553

554 - function resetWorkPriority(workInProgress: Fiber) {

554 + function resetWorkPriority(

555 + workInProgress: Fiber,

556 + renderPriority: PriorityLevel,

557 + ) {

558 + if (

559 + workInProgress.pendingWorkPriority !== NoWork &&

560 + workInProgress.pendingWorkPriority > renderPriority

561 + ) {

562 + // This was a down-prioritization. Don't bubble priority from children.

563 + return;

564 + }

565 +

566 // Check for pending update priority.

567 let newPriority = getUpdatePriority(workInProgress);

568

@@ -560,12 +571,7 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(

571 let child = workInProgress.child;

572 while (child !== null) {

573 // Ensure that remaining work priority bubbles up.

563 - if (

564 - child.pendingWorkPriority !== NoWork &&

565 - (newPriority === NoWork || newPriority > child.pendingWorkPriority)

566 - ) {

567 - newPriority = child.pendingWorkPriority;

568 - }

574 + newPriority = largerPriority(newPriority, child.pendingWorkPriority);

575 child = child.sibling;

576 }

577 workInProgress.pendingWorkPriority = newPriority;

@@ -578,12 +584,12 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(

584 // means that we don't need an additional field on the work in

585 // progress.

586 const current = workInProgress.alternate;

581 - const next = completeWork(current, workInProgress);

587 + const next = completeWork(current, workInProgress, nextPriorityLevel);

588

589 const returnFiber = workInProgress.return;

590 const siblingFiber = workInProgress.sibling;

591

586 - resetWorkPriority(workInProgress);

592 + resetWorkPriority(workInProgress, nextPriorityLevel);

593

594 if (next !== null) {

595 if (__DEV__) {

src/renderers/shared/fiber/__tests__/ReactIncremental-test.js

+3 -3

@@ -244,7 +244,7 @@ describe('ReactIncremental', () => {

244 expect(inst.state).toEqual({text: 'bar', text2: 'baz'});

245 });

246

247 - xit('can deprioritize unfinished work and resume it later', () => {

247 + it('can deprioritize unfinished work and resume it later', () => {

248 var ops = [];

249

250 function Bar(props) {

@@ -296,7 +296,7 @@ describe('ReactIncremental', () => {

296 expect(ops).toEqual(['Middle', 'Middle']);

297 });

298

299 - xit('can deprioritize a tree from without dropping work', () => {

299 + it('can deprioritize a tree from without dropping work', () => {

300 var ops = [];

301

302 function Bar(props) {

@@ -1962,7 +1962,7 @@ describe('ReactIncremental', () => {

1962 ]);

1963 });

1964

1965 - xit('provides context when reusing work', () => {

1965 + it('provides context when reusing work', () => {

1966 var ops = [];

1967

1968 class Intl extends React.Component {

src/renderers/shared/fiber/__tests__/ReactIncrementalPerf-test.js

+1 -1

@@ -280,7 +280,7 @@ describe('ReactDebugFiberPerf', () => {

280 expect(getFlameChart()).toMatchSnapshot();

281 });

282

283 - xit('measures deprioritized work', () => {

283 + it('measures deprioritized work', () => {

284 addComment('Flush the parent');

285 ReactNoop.syncUpdates(() => {

286 ReactNoop.render(

src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js

+82 -85

@@ -691,109 +691,106 @@ describe('ReactIncrementalSideEffects', () => {

691 expect(ops).toEqual(['Bar', 'Baz', 'Bar', 'Bar']);

692 });

693

694 - xit(

695 - 'deprioritizes setStates that happens within a deprioritized tree',

696 - () => {

697 - var ops = [];

698 -

699 - var barInstances = [];

700 -

701 - class Bar extends React.Component {

702 - constructor() {

703 - super();

704 - this.state = {active: false};

705 - barInstances.push(this);

706 - }

707 - activate() {

708 - this.setState({active: true});

709 - }

710 - render() {

711 - ops.push('Bar');

712 - return <span prop={this.state.active ? 'X' : this.props.idx} />;

713 - }

694 + it('deprioritizes setStates that happens within a deprioritized tree', () => {

695 + var ops = [];

696 +

697 + var barInstances = [];

698 +

699 + class Bar extends React.Component {

700 + constructor() {

701 + super();

702 + this.state = {active: false};

703 + barInstances.push(this);

704 }

715 - function Foo(props) {

716 - ops.push('Foo');

717 - return (

718 - <div>

719 - <span prop={props.tick} />

720 - <div hidden={true}>

721 - <Bar idx={props.idx} />

722 - <Bar idx={props.idx} />

723 - <Bar idx={props.idx} />

724 - </div>

725 - </div>

726 - );

705 + activate() {

706 + this.setState({active: true});

707 + }

708 + render() {

709 + ops.push('Bar');

710 + return <span prop={this.state.active ? 'X' : this.props.idx} />;

711 }

728 - ReactNoop.render(<Foo tick={0} idx={0} />);

729 - ReactNoop.flush();

730 - expect(ReactNoop.getChildren()).toEqual([

731 - div(span(0), div(span(0), span(0), span(0))),

732 - ]);

712 + }

713 + function Foo(props) {

714 + ops.push('Foo');

715 + return (

716 + <div>

717 + <span prop={props.tick} />

718 + <div hidden={true}>

719 + <Bar idx={props.idx} />

720 + <Bar idx={props.idx} />

721 + <Bar idx={props.idx} />

722 + </div>

723 + </div>

724 + );

725 + }

726 + ReactNoop.render(<Foo tick={0} idx={0} />);

727 + ReactNoop.flush();

728 + expect(ReactNoop.getChildren()).toEqual([

729 + div(span(0), div(span(0), span(0), span(0))),

730 + ]);

731

734 - expect(ops).toEqual(['Foo', 'Bar', 'Bar', 'Bar']);

732 + expect(ops).toEqual(['Foo', 'Bar', 'Bar', 'Bar']);

733

736 - ops = [];

734 + ops = [];

735

738 - ReactNoop.render(<Foo tick={1} idx={1} />);

739 - ReactNoop.flushDeferredPri(70 + 5);

740 - expect(ReactNoop.getChildren()).toEqual([

736 + ReactNoop.render(<Foo tick={1} idx={1} />);

737 + ReactNoop.flushDeferredPri(70 + 5);

738 + expect(ReactNoop.getChildren()).toEqual([

739 + div(

740 + // Updated.

741 + span(1),

742 div(

742 - // Updated.

743 - span(1),

744 - div(

745 - // Still not updated.

746 - span(0),

747 - span(0),

748 - span(0),

749 - ),

743 + // Still not updated.

744 + span(0),

745 + span(0),

746 + span(0),

747 ),

751 - ]);

748 + ),

749 + ]);

750

753 - expect(ops).toEqual(['Foo', 'Bar', 'Bar']);

754 - ops = [];

751 + expect(ops).toEqual(['Foo', 'Bar', 'Bar']);

752 + ops = [];

753

756 - barInstances[0].activate();

754 + barInstances[0].activate();

755

758 - // This should not be enough time to render the content of all the hidden

759 - // items. Including the set state since that is deprioritized.

760 - // TODO: The cycles it takes to do this could be lowered with further

761 - // optimizations.

762 - ReactNoop.flushDeferredPri(60 + 5);

763 - expect(ReactNoop.getChildren()).toEqual([

756 + // This should not be enough time to render the content of all the hidden

757 + // items. Including the set state since that is deprioritized.

758 + // TODO: The cycles it takes to do this could be lowered with further

759 + // optimizations.

760 + ReactNoop.flushDeferredPri(60 + 5);

761 + expect(ReactNoop.getChildren()).toEqual([

762 + div(

763 + // Updated.

764 + span(1),

765 div(

765 - // Updated.

766 - span(1),

767 - div(

768 - // Still not updated.

769 - span(0),

770 - span(0),

771 - span(0),

772 - ),

766 + // Still not updated.

767 + span(0),

768 + span(0),

769 + span(0),

770 ),

774 - ]);

771 + ),

772 + ]);

773

776 - expect(ops).toEqual(['Bar']);

777 - ops = [];

774 + expect(ops).toEqual(['Bar']);

775 + ops = [];

776

779 - // However, once we render fully, we will have enough time to finish it all

780 - // at once.

781 - ReactNoop.flush();

782 - expect(ReactNoop.getChildren()).toEqual([

777 + // However, once we render fully, we will have enough time to finish it all

778 + // at once.

779 + ReactNoop.flush();

780 + expect(ReactNoop.getChildren()).toEqual([

781 + div(

782 + span(1),

783 div(

784 + // Now we had enough time to finish the spans.

785 + span('X'),

786 + span(1),

787 span(1),

785 - div(

786 - // Now we had enough time to finish the spans.

787 - span('X'),

788 - span(1),

789 - span(1),

790 - ),

788 ),

792 - ]);

789 + ),

790 + ]);

791

794 - expect(ops).toEqual(['Bar']);

795 - },

796 - );

792 + expect(ops).toEqual(['Bar', 'Bar']);

793 + });

794 // TODO: Test that side-effects are not cut off when a work in progress node

795 // moves to "current" without flushing due to having lower priority. Does this

796 // even happen? Maybe a child doesn't get processed because it is lower prio?