@samitouri / QOS-React / commits / 5e65f2f622

Bubble up pending work priority to the top level

This is a bit poorly structured. I'll restructure when the pieces are better in place. Basically we reset the priority of a node before work on the children. The children then bump their parent if they end up having work left. This is the first time we're seeing deep updates happening. The new unit test demonstrates this. There is an interesting case that happens when we fall back out of a deep update. We end up "completing" a node that we didn't begin. This probably breaks in coroutines. When that completes, it'll try to render the sibling next but that should bail out so we check for any pending work on the sibling. That one I'm not sure about.

Sebastian Markbage committed Jun 14, 2016 at 19:19 UTC 5e65f2f622d2e136a005d6254c84fdb3d4f95cca
4 files changed +131 -24
src/renderers/shared/fiber/ReactFiber.js
+1
@@ -193,5 +193,6 @@ exports.createFiberFromCoroutine = function(coroutine : ReactCoroutine, priority
193
194 exports.createFiberFromYield = function(yieldNode : ReactYield, priorityLevel : PriorityLevel) {
195 const fiber = createFiber(YieldComponent, yieldNode.key);
196 + fiber.pendingProps = {};
197 return fiber;
198 };
src/renderers/shared/fiber/ReactFiberBeginWork.js
+15
@@ -28,6 +28,7 @@ var {
28 YieldComponent,
29 } = ReactTypeOfWork;
30 var {
31 + NoWork,
32 OffscreenPriority,
33 } = require('ReactPriorityLevel');
34
@@ -47,6 +48,7 @@ function updateFunctionalComponent(current, workInProgress) {
48 console.log('update fn:', fn.name);
49 var nextChildren = fn(props);
50 reconcileChildren(current, workInProgress, nextChildren);
51 + workInProgress.pendingWorkPriority = NoWork;
52 }
53
54 function updateHostComponent(current, workInProgress) {
@@ -65,6 +67,7 @@ function updateHostComponent(current, workInProgress) {
67 nextChildren,
68 OffscreenPriority
69 );
70 + workInProgress.pendingWorkPriority = OffscreenPriority;
71 return null;
72 } else {
73 workInProgress.child = ReactChildFiber.reconcileChildFibers(
@@ -73,6 +76,7 @@ function updateHostComponent(current, workInProgress) {
76 nextChildren,
77 priority
78 );
79 + workInProgress.pendingWorkPriority = NoWork;
80 return workInProgress.child;
81 }
82 }
@@ -85,12 +89,19 @@ function mountIndeterminateComponent(current, workInProgress) {
89 console.log('performed work on class:', fn.name);
90 // Proceed under the assumption that this is a class instance
91 workInProgress.tag = ClassComponent;
92 + if (workInProgress.alternate) {
93 + workInProgress.alternate.tag = ClassComponent;
94 + }
95 } else {
96 console.log('performed work on fn:', fn.name);
97 // Proceed under the assumption that this is a functional component
98 workInProgress.tag = FunctionalComponent;
99 + if (workInProgress.alternate) {
100 + workInProgress.alternate.tag = FunctionalComponent;
101 + }
102 }
103 reconcileChildren(current, workInProgress, value);
104 + workInProgress.pendingWorkPriority = NoWork;
105 }
106
107 function updateCoroutineComponent(current, workInProgress) {
@@ -100,6 +111,7 @@ function updateCoroutineComponent(current, workInProgress) {
111 }
112 console.log('begin coroutine', workInProgress.type.name);
113 reconcileChildren(current, workInProgress, coroutine.children);
114 + workInProgress.pendingWorkPriority = NoWork;
115 }
116
117 function beginWork(current : ?Fiber, workInProgress : Fiber) : ?Fiber {
@@ -114,6 +126,7 @@ function beginWork(current : ?Fiber, workInProgress : Fiber) : ?Fiber {
126 workInProgress.output = current.output;
127 workInProgress.child = current.child;
128 workInProgress.stateNode = current.stateNode;
129 + workInProgress.pendingWorkPriority = NoWork;
130 return null;
131 }
132 if (workInProgress.pendingProps === workInProgress.memoizedProps) {
@@ -135,6 +148,7 @@ function beginWork(current : ?Fiber, workInProgress : Fiber) : ?Fiber {
148 reconcileChildren(current, workInProgress, workInProgress.pendingProps);
149 // A yield component is just a placeholder, we can just run through the
150 // next one immediately.
151 + workInProgress.pendingWorkPriority = NoWork;
152 if (workInProgress.child) {
153 return beginWork(
154 workInProgress.child.alternate,
@@ -162,6 +176,7 @@ function beginWork(current : ?Fiber, workInProgress : Fiber) : ?Fiber {
176 case YieldComponent:
177 // A yield component is just a placeholder, we can just run through the
178 // next one immediately.
179 + workInProgress.pendingWorkPriority = NoWork;
180 if (workInProgress.sibling) {
181 return beginWork(
182 workInProgress.sibling.alternate,
src/renderers/shared/fiber/ReactFiberReconciler.js
+65 -23
@@ -65,7 +65,7 @@ module.exports = function<T, P, I>(config : HostConfig<T, P, I>) : Reconciler {
65
66 let currentRootsWithPendingWork : ?Fiber = null;
67
68 - function findNextUnitOfWork(priorityLevel : PriorityLevel) : ?Fiber {
68 + function findNextUnitOfWorkAtPriority(priorityLevel : PriorityLevel) : ?Fiber {
69 let current = currentRootsWithPendingWork;
70 while (current) {
71 if (current.pendingWorkPriority !== 0 &&
@@ -74,11 +74,15 @@ module.exports = function<T, P, I>(config : HostConfig<T, P, I>) : Reconciler {
74 if (current.pendingProps !== null) {
75 // We found some work to do. We need to return the "work in progress"
76 // of this node which will be the alternate.
77 - return current.alternate;
77 + const clone = ReactFiber.cloneFiber(current, current.pendingWorkPriority);
78 + clone.pendingProps = current.pendingProps;
79 + return clone;
80 }
81 // If we have a child let's see if any of our children has work to do.
82 // Only bother doing this at all if the current priority level matches
83 // because it is the highest priority for the whole subtree.
84 + // TODO: Coroutines can have work in their stateNode which is another
85 + // type of child that needs to be searched for work.
86 if (current.child) {
87 current = current.child;
88 continue;
@@ -106,6 +110,21 @@ module.exports = function<T, P, I>(config : HostConfig<T, P, I>) : Reconciler {
110 return null;
111 }
112
113 + function findNextUnitOfWork() {
114 + // Find the highest possible priority work to do.
115 + // This loop is unrolled just to satisfy Flow's enum constraint.
116 + // We could make arbitrary many idle priority levels but having
117 + // too many just means flushing changes too often.
118 + let work = findNextUnitOfWorkAtPriority(HighPriority);
119 + if (!work) {
120 + work = findNextUnitOfWorkAtPriority(LowPriority);
121 + if (!work) {
122 + work = findNextUnitOfWorkAtPriority(OffscreenPriority);
123 + }
124 + }
125 + return work;
126 + }
127 +
128 function completeUnitOfWork(workInProgress : Fiber) : ?Fiber {
129 while (true) {
130 // The current, flushed, state of this fiber is the alternate.
@@ -114,29 +133,62 @@ module.exports = function<T, P, I>(config : HostConfig<T, P, I>) : Reconciler {
133 // progress.
134 const current = workInProgress.alternate;
135 const next = completeWork(current, workInProgress);
136 +
137 + // The work is now done. We don't need this anymore. This flags
138 + // to the system not to redo any work here.
139 + workInProgress.pendingProps = null;
140 +
141 + // TODO: Stop using the parent for this purpose. I think this will break
142 + // down in edge cases because when nodes are reused during bailouts, we
143 + // don't know which of two parents was used. Instead we should maintain
144 + // a temporary manual stack.
145 + // $FlowFixMe: This downcast is not safe. It is intentionally an error.
146 + const parent = workInProgress.parent;
147 +
148 + // Ensure that remaining work priority bubbles up.
149 + if (parent && workInProgress.pendingWorkPriority !== NoWork &&
150 + (parent.pendingWorkPriority === NoWork ||
151 + parent.pendingWorkPriority > workInProgress.pendingWorkPriority)) {
152 + parent.pendingWorkPriority = workInProgress.pendingWorkPriority;
153 + }
154 +
155 if (next) {
156 // If completing this work spawned new work, do that next.
157 return next;
158 } else if (workInProgress.sibling) {
159 // If there is more work to do in this parent, do that next.
160 return workInProgress.sibling;
123 - } else if (workInProgress.parent) {
161 + } else if (parent) {
162 // If there's no more work in this parent. Complete the parent.
125 - // TODO: Stop using the parent for this purpose. I think this will break
126 - // down in edge cases because when nodes are reused during bailouts, we
127 - // don't know which of two parents was used. Instead we should maintain
128 - // a temporary manual stack.
129 - // $FlowFixMe: This downcast is not safe. It is intentionally an error.
130 - workInProgress = workInProgress.parent;
163 + workInProgress = parent;
164 } else {
165 // If we're at the root, there's no more work to do.
133 - currentRootsWithPendingWork = null;
134 - return null;
166 + console.log('completed flush with remaining work at priority', workInProgress.pendingWorkPriority);
167 + if (workInProgress.pendingWorkPriority !== NoWork) {
168 + // TODO: This removes all but one node. Broken.
169 + currentRootsWithPendingWork = workInProgress;
170 + const nextWork = findNextUnitOfWork();
171 + if (!nextWork) {
172 + // Something went wrong and there wasn't actually any more work.
173 + // TODO: This currently fails because of the parent/root hacks.
174 + // throw new Error('Should never finish with a priority level unless there is work.');
175 + currentRootsWithPendingWork = null;
176 + return null;
177 + }
178 + return nextWork;
179 + } else {
180 + currentRootsWithPendingWork = null;
181 + return null;
182 + }
183 }
184 }
185 }
186
187 function performUnitOfWork(workInProgress : Fiber) : ?Fiber {
188 + // Ignore work if there is nothing to do.
189 + if (workInProgress.pendingProps === null) {
190 + return null;
191 + }
192 // The current, flushed, state of this fiber is the alternate.
193 // Ideally nothing should rely on this, but relying on it here
194 // means that we don't need an additional field on the work in
@@ -154,17 +206,7 @@ module.exports = function<T, P, I>(config : HostConfig<T, P, I>) : Reconciler {
206
207 function performLowPriWork(deadline : Deadline) {
208 if (!nextUnitOfWork) {
157 - // Find the highest possible priority work to do.
158 - // This loop is unrolled just to satisfy Flow's enum constraint.
159 - // We could make arbitrary many idle priority levels but having
160 - // too many just means flushing changes too often.
161 - nextUnitOfWork = findNextUnitOfWork(HighPriority);
162 - if (!nextUnitOfWork) {
163 - nextUnitOfWork = findNextUnitOfWork(LowPriority);
164 - if (!nextUnitOfWork) {
165 - nextUnitOfWork = findNextUnitOfWork(OffscreenPriority);
166 - }
167 - }
209 + nextUnitOfWork = findNextUnitOfWork();
210 }
211 while (nextUnitOfWork) {
212 if (deadline.timeRemaining() > timeHeuristicForUnitOfWork) {
@@ -236,7 +278,7 @@ module.exports = function<T, P, I>(config : HostConfig<T, P, I>) : Reconciler {
278 },
279
280 unmountContainer(container : OpaqueNode) : void {
239 - container.pendingProps = null;
281 + container.pendingProps = [];
282 container.pendingWorkPriority = LowPriority;
283
284 scheduleLowPriWork(container);
src/renderers/shared/fiber/__tests__/ReactIncremental-test.js
+50 -1
@@ -161,7 +161,56 @@ describe('ReactIncremental', function() {
161
162 expect(ops).toEqual(['Foo', 'Bar', 'Bar']);
163
164 - // TODO: Test the ability for a subtree to resume if it has lower priority.
164 + });
165 +
166 + it('can deprioritize unfinished work and resume it later', function() {
167 +
168 + var ops = [];
169 +
170 + function Bar(props) {
171 + ops.push('Bar');
172 + return <div>{props.children}</div>;
173 + }
174 +
175 + function Middle(props) {
176 + ops.push('Middle');
177 + return <span>{props.children}</span>;
178 + }
179 +
180 + function Foo(props) {
181 + ops.push('Foo');
182 + return (
183 + <div>
184 + <Bar>{props.text}</Bar>
185 + <div hidden={true}>
186 + <span>
187 + <Middle>{props.text}</Middle>
188 + </span>
189 + </div>
190 + <Bar>{props.text}</Bar>
191 + </div>
192 + );
193 + }
194 +
195 + // Init
196 + ReactNoop.render(<Foo text="foo" />);
197 + ReactNoop.flush();
198 +
199 + ops = [];
200 +
201 + // Render part of the work. This should be enough to flush everything except
202 + // the middle which has lower priority.
203 + ReactNoop.render(<Foo text="bar" />);
204 + ReactNoop.flushLowPri(40);
205 +
206 + expect(ops).toEqual(['Foo', 'Bar', 'Bar']);
207 +
208 + ops = [];
209 +
210 + // Flush only the remaining work
211 + ReactNoop.flush();
212 +
213 + expect(ops).toEqual(['Middle']);
214
215 });
216