Separate priority field for pending updates

Currently there is a single priority field `pendingWorkPriority` that
represents the priority of the entire subtree. The scheduler does a
breadth-first search to find the next unit of work with a matching
priority level. If a subtree's priority does not match, that entire
subtree is skipped. That means when an update is scheduled deep in the
tree (`setState`), its priority must be bubbled to the top of the tree.
The problem is that this causes the all the parent priorities to be
overridden — there's no way to schedule a high priority update inside
a low priority tree without increasing the priority of the entire tree.

To address this, this PR introduces a separate priority field called
`pendingUpdatePriority` (not 100% sold on the name). The new field
represents the priority of the pending props and state, which may be
lower than the priority of the entire tree. Now, when the scheduler is
searching for work to perform, it only matches if the `
pendingUpdatePriority` matches. It continues to use the
`pendingWorkPriority` as a way to ignore subtrees if they do not match.
(To elaborate further: because the work priority is the highest priority
 of any node in the entire tree, if it does not match, the tree can
be skipped.)

Author: acdlite

src/renderers/shared/fiber/ReactChildFiber.js

@@ -26,6 +26,7 @@ var {
 
 var ReactFiber = require('ReactFiber');
 var ReactReifiedYield = require('ReactReifiedYield');
+var ReactPriorityLevel = require('ReactPriorityLevel');
 
 const {
   cloneFiber,
@@ -38,6 +39,10 @@ const {
   createReifiedYield,
 } = ReactReifiedYield;
 
+const {
+  NoWork,
+} = ReactPriorityLevel;
+
 const isArray = Array.isArray;
 
 function ChildReconciler(shouldClone) {
@@ -64,7 +69,11 @@ function ChildReconciler(shouldClone) {
           const clone = shouldClone ? cloneFiber(existingChild, priority) : existingChild;
           if (!shouldClone) {
             // TODO: This might be lowering the priority of nested unfinished work.
-            clone.pendingWorkPriority = priority;
+            clone.pendingUpdatePriority = priority;
+            if (clone.pendingWorkPriority === NoWork ||
+                clone.pendingWorkPriority > priority) {
+              clone.pendingWorkPriority = priority;
+            }
           }
           clone.pendingProps = element.props;
           // clone.child = existingChild.child;
@@ -136,7 +145,11 @@ function ChildReconciler(shouldClone) {
           const clone = shouldClone ? cloneFiber(existingChild, priority) : existingChild;
           if (!shouldClone) {
             // TODO: This might be lowering the priority of nested unfinished work.
-            clone.pendingWorkPriority = priority;
+            clone.pendingUpdatePriority = priority;
+            if (clone.pendingWorkPriority === NoWork ||
+                clone.pendingWorkPriority > priority) {
+              clone.pendingWorkPriority = priority;
+            }
           }
           clone.pendingProps = element.props;
           // clone.child = existingChild.child;

src/renderers/shared/fiber/ReactFiber.js

@@ -96,7 +96,12 @@ export type Fiber = Instance & {
   firstEffect: ?Fiber,
   lastEffect: ?Fiber,
 
-  // This will be used to quickly determine if a subtree has no pending changes.
+  // The update priority is the priority of a fiber's pending props and state.
+  // It may be lower than the priority of the entire subtree.
+  pendingUpdatePriority: PriorityLevel,
+
+  // The work priority is the priority of the entire subtree. It will be used to
+  // quickly determine if a subtree has no pending changes.
   pendingWorkPriority: PriorityLevel,
 
   // This value represents the priority level that was last used to process this
@@ -167,6 +172,7 @@ var createFiber = function(tag : TypeOfWork, key : null | string) : Fiber {
     firstEffect: null,
     lastEffect: null,
 
+    pendingUpdatePriority: NoWork,
     pendingWorkPriority: NoWork,
     progressedPriority: NoWork,
     progressedChild: null,
@@ -191,6 +197,28 @@ exports.cloneFiber = function(fiber : Fiber, priorityLevel : PriorityLevel) : Fi
   // fiber.progressedPriority = NoWork;
   // fiber.progressedChild = null;
 
+  // Don't deprioritize when cloning. Unlike other priority comparisons (e.g.
+  // in the scheduler), this one must check that priorityLevel is not equal to
+  // NoWork, otherwise work will be dropped. For complete correctness, the other
+  // priority comparisons should also perform this check, even though it's not
+  // an issue in practice. I didn't catch this at first and it created a subtle
+  // bug, which suggests we may need to extract the logic into a
+  // utility function (shouldOverridePriority).
+  let updatePriority;
+  let workPriority;
+  if (priorityLevel !== NoWork &&
+      (priorityLevel < fiber.pendingUpdatePriority || fiber.pendingUpdatePriority === NoWork)) {
+    updatePriority = priorityLevel;
+  } else {
+    updatePriority = fiber.pendingUpdatePriority;
+  }
+  if (updatePriority !== NoWork &&
+      (updatePriority < fiber.pendingWorkPriority || fiber.pendingWorkPriority === NoWork)) {
+    workPriority = updatePriority;
+  } else {
+    workPriority = fiber.pendingWorkPriority;
+  }
+
   // We use a double buffering pooling technique because we know that we'll only
   // ever need at most two versions of a tree. We pool the "other" unused node
   // that we're free to reuse. This is lazily created to avoid allocating extra
@@ -204,7 +232,8 @@ exports.cloneFiber = function(fiber : Fiber, priorityLevel : PriorityLevel) : Fi
     alt.pendingProps = fiber.pendingProps; // TODO: Pass as argument.
     alt.updateQueue = fiber.updateQueue;
     alt.callbackList = fiber.callbackList;
-    alt.pendingWorkPriority = priorityLevel;
+    alt.pendingUpdatePriority = updatePriority;
+    alt.pendingWorkPriority = workPriority;
 
     alt.child = fiber.child;
     alt.memoizedProps = fiber.memoizedProps;
@@ -231,7 +260,8 @@ exports.cloneFiber = function(fiber : Fiber, priorityLevel : PriorityLevel) : Fi
   alt.pendingProps = fiber.pendingProps;
   alt.updateQueue = fiber.updateQueue;
   alt.callbackList = fiber.callbackList;
-  alt.pendingWorkPriority = priorityLevel;
+  alt.pendingUpdatePriority = updatePriority;
+  alt.pendingWorkPriority = workPriority;
 
   alt.memoizedProps = fiber.memoizedProps;
   alt.output = fiber.output;
@@ -253,6 +283,7 @@ exports.createFiberFromElement = function(element : ReactElement<*>, priorityLev
 // $FlowFixMe: ReactElement.key is currently defined as ?string but should be defined as null | string in Flow.
   const fiber = createFiberFromElementType(element.type, element.key);
   fiber.pendingProps = element.props;
+  fiber.pendingUpdatePriority = priorityLevel;
   fiber.pendingWorkPriority = priorityLevel;
   return fiber;
 };
@@ -282,6 +313,7 @@ exports.createFiberFromCoroutine = function(coroutine : ReactCoroutine, priority
   const fiber = createFiber(CoroutineComponent, coroutine.key);
   fiber.type = coroutine.handler;
   fiber.pendingProps = coroutine;
+  fiber.pendingUpdatePriority = priorityLevel;
   fiber.pendingWorkPriority = priorityLevel;
   return fiber;
 };

src/renderers/shared/fiber/ReactFiberBeginWork.js

@@ -64,7 +64,7 @@ module.exports = function<T, P, I, C>(config : HostConfig<T, P, I, C>, getSchedu
   }
 
   function reconcileChildren(current, workInProgress, nextChildren) {
-    const priorityLevel = workInProgress.pendingWorkPriority;
+    const priorityLevel = workInProgress.pendingUpdatePriority;
     reconcileChildrenAtPriority(current, workInProgress, nextChildren, priorityLevel);
   }
 
@@ -124,14 +124,29 @@ module.exports = function<T, P, I, C>(config : HostConfig<T, P, I, C>, getSchedu
     if (fiber.alternate) {
       fiber.alternate.updateQueue = updateQueue;
     }
+
+    // Set the update priority of the fiber and its alternate
+    if (fiber.pendingUpdatePriority === NoWork ||
+        fiber.pendingUpdatePriority > priorityLevel) {
+      fiber.pendingUpdatePriority = priorityLevel;
+    }
+    if (fiber.alternate) {
+      if (fiber.alternate.pendingUpdatePriority === NoWork ||
+          fiber.alternate.pendingUpdatePriority > priorityLevel) {
+        fiber.alternate.pendingUpdatePriority = priorityLevel;
+      }
+    }
+
+    // For this fiber and all its ancestors and their alternates, set the
+    // work (subtree) priority
     while (true) {
       if (fiber.pendingWorkPriority === NoWork ||
-          fiber.pendingWorkPriority >= priorityLevel) {
+          fiber.pendingWorkPriority > priorityLevel) {
         fiber.pendingWorkPriority = priorityLevel;
       }
       if (fiber.alternate) {
         if (fiber.alternate.pendingWorkPriority === NoWork ||
-            fiber.alternate.pendingWorkPriority >= priorityLevel) {
+            fiber.alternate.pendingWorkPriority > priorityLevel) {
           fiber.alternate.pendingWorkPriority = priorityLevel;
         }
       }
@@ -235,7 +250,7 @@ module.exports = function<T, P, I, C>(config : HostConfig<T, P, I, C>, getSchedu
   function updateHostComponent(current, workInProgress) {
     const nextChildren = workInProgress.pendingProps.children;
     if (workInProgress.pendingProps.hidden &&
-        workInProgress.pendingWorkPriority !== OffscreenPriority) {
+        workInProgress.pendingUpdatePriority !== OffscreenPriority) {
       // If this host component is hidden, we can bail out on the children.
       // We'll rerender the children later at the lower priority.
 
@@ -311,7 +326,7 @@ module.exports = function<T, P, I, C>(config : HostConfig<T, P, I, C>, getSchedu
   */
 
   function bailoutOnAlreadyFinishedWork(current, workInProgress : Fiber) : ?Fiber {
-    const priorityLevel = workInProgress.pendingWorkPriority;
+    const priorityLevel = workInProgress.pendingUpdatePriority;
 
     // TODO: We should ideally be able to bail out early if the children have no
     // more work to do. However, since we don't have a separation of this

src/renderers/shared/fiber/ReactFiberCompleteWork.js

@@ -113,7 +113,7 @@ module.exports = function<T, P, I, C>(config : HostConfig<T, P, I, C>) {
 
     var currentFirstChild = current ? current.stateNode : null;
     // Inherit the priority of the returnFiber.
-    const priority = workInProgress.pendingWorkPriority;
+    const priority = workInProgress.pendingUpdatePriority;
     workInProgress.stateNode = reconcileChildFibers(
       workInProgress,
       currentFirstChild,

src/renderers/shared/fiber/ReactFiberReconciler.js

@@ -73,6 +73,7 @@ module.exports = function<T, P, I, C>(config : HostConfig<T, P, I, C>) : Reconci
       // TODO: This should not override the pendingWorkPriority if there is
       // higher priority work in the subtree.
       container.pendingProps = element;
+      container.pendingUpdatePriority = LowPriority;
       container.pendingWorkPriority = LowPriority;
 
       scheduleLowPriWork(root, LowPriority);
@@ -88,6 +89,7 @@ module.exports = function<T, P, I, C>(config : HostConfig<T, P, I, C>) : Reconci
       const root : FiberRoot = (container.stateNode : any);
       // TODO: Use pending work/state instead of props.
       root.current.pendingProps = element;
+      root.current.pendingUpdatePriority = LowPriority;
       root.current.pendingWorkPriority = LowPriority;
 
       scheduleLowPriWork(root, LowPriority);
@@ -98,6 +100,7 @@ module.exports = function<T, P, I, C>(config : HostConfig<T, P, I, C>) : Reconci
       const root : FiberRoot = (container.stateNode : any);
       // TODO: Use pending work/state instead of props.
       root.current.pendingProps = [];
+      root.current.pendingUpdatePriority = LowPriority;
       root.current.pendingWorkPriority = LowPriority;
 
       scheduleLowPriWork(root, LowPriority);

src/renderers/shared/fiber/ReactFiberScheduler.js

@@ -75,8 +75,8 @@ module.exports = function<T, P, I, C>(config : HostConfig<T, P, I, C>) {
     let highestPriorityLevel = NoWork;
     while (root) {
       if (highestPriorityLevel === NoWork ||
-          highestPriorityLevel > root.current.pendingWorkPriority) {
-        highestPriorityLevel = root.current.pendingWorkPriority;
+          highestPriorityLevel > root.current.pendingUpdatePriority) {
+        highestPriorityLevel = root.current.pendingUpdatePriority;
         highestPriorityRoot = root;
       }
       // We didn't find anything to do in this root, so let's try the next one.
@@ -137,6 +137,7 @@ module.exports = function<T, P, I, C>(config : HostConfig<T, P, I, C>) {
       const current = workInProgress.alternate;
       const next = completeWork(current, workInProgress);
 
+      workInProgress.pendingUpdatePriority = NoWork;
       resetWorkPriority(workInProgress);
 
       // The work is now done. We don't need this anymore. This flags

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

@@ -692,6 +692,51 @@ describe('ReactIncremental', function() {
     expect(instance.state.called).toEqual(true);
   });
 
+  it('can setState without overriding its parents\' priority', () => {
+    let ops = [];
+    let instance;
+    class Baz extends React.Component {
+      constructor() {
+        super();
+        instance = this;
+        this.state = { num: 0 };
+      }
+      render() {
+        ops.push('Baz');
+        return <span />;
+      }
+    }
+
+    function Bar({ id }) {
+      ops.push('Bar' + id);
+      return <div />;
+    }
+
+    function Foo() {
+      ops.push('Foo');
+      return [
+        <section hidden={true}>
+          <Bar id={1} />
+          <Baz />
+        </section>,
+        <Bar id={2} />,
+      ];
+    }
+
+    ReactNoop.render(<Foo />);
+    ReactNoop.flush();
+    expect(ops).toEqual(['Foo', 'Bar2', 'Bar1', 'Baz']);
+    ops = [];
+    ReactNoop.render(<Foo />);
+    // Even though Baz is in a hidden subtree, calling setState gives it a
+    // higher priority. It should not affect the priority of anything else in
+    // the subtree.
+    instance.setState({ num: 1 });
+    ReactNoop.flush();
+    // Baz should come before Bar1 because it has higher priority
+    expect(ops).toEqual(['Foo', 'Bar2', 'Baz', 'Bar1']);
+  });
+
   it('can replaceState', () => {
     let instance;
     const Bar = React.createClass({