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

@@ -28,6 +28,7 @@ var {
 
 var ReactFiber = require('ReactFiber');
 var ReactReifiedYield = require('ReactReifiedYield');
+var ReactPriorityLevel = require('ReactPriorityLevel');
 
 const {
   cloneFiber,
@@ -40,6 +41,10 @@ const {
   createReifiedYield,
 } = ReactReifiedYield;
 
+const {
+  NoWork,
+} = ReactPriorityLevel;
+
 const isArray = Array.isArray;
 
 function ChildReconciler(shouldClone) {
@@ -66,7 +71,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;
@@ -134,7 +143,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

@@ -99,7 +99,11 @@ export type Fiber = Instance & {
   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 is a pooled version of a Fiber. Every fiber that gets updated will
@@ -150,6 +154,7 @@ var createFiber = function(tag : TypeOfWork, key : null | string) : Fiber {
     firstEffect: null,
     lastEffect: null,
 
+    pendingUpdatePriority: NoWork,
     pendingWorkPriority: NoWork,
 
     childInProgress: null,
@@ -165,6 +170,28 @@ function shouldConstruct(Component) {
 
 // This is used to create an alternate fiber to do work on.
 exports.cloneFiber = function(fiber : Fiber, priorityLevel : PriorityLevel) : Fiber {
+  // 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.pendingWorkPriority;
+  }
+  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
@@ -180,7 +207,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;
@@ -206,7 +234,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;
@@ -224,6 +253,7 @@ exports.createHostContainerFiber = function() {
 exports.createFiberFromElement = function(element : ReactElement, priorityLevel : PriorityLevel) {
   const fiber = createFiberFromElementType(element.type, element.key);
   fiber.pendingProps = element.props;
+  fiber.pendingUpdatePriority = priorityLevel;
   fiber.pendingWorkPriority = priorityLevel;
   return fiber;
 };
@@ -253,6 +283,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

@@ -53,7 +53,7 @@ module.exports = function<T, P, I, C>(config : HostConfig<T, P, I, C>, getSchedu
   function reconcileChildren(current, workInProgress, nextChildren) {
     // TODO: Children needs to be able to reconcile in place if we are
     // overriding progressed work.
-    const priority = workInProgress.pendingWorkPriority;
+    const priority = workInProgress.pendingUpdatePriority;
     reconcileChildrenAtPriority(current, workInProgress, nextChildren, priority);
   }
 
@@ -99,14 +99,29 @@ module.exports = function<T, P, I, C>(config : HostConfig<T, P, I, C>, getSchedu
     if (fiber.alternate !== null) {
       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 !== null) {
+      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 !== null) {
         if (fiber.alternate.pendingWorkPriority === NoWork ||
-            fiber.alternate.pendingWorkPriority >= priorityLevel) {
+            fiber.alternate.pendingWorkPriority > priorityLevel) {
           fiber.alternate.pendingWorkPriority = priorityLevel;
         }
       }
@@ -216,7 +231,7 @@ module.exports = function<T, P, I, C>(config : HostConfig<T, P, I, C>, getSchedu
   function updateHostComponent(current, workInProgress) {
     var nextChildren = workInProgress.pendingProps.children;
 
-    let priority = workInProgress.pendingWorkPriority;
+    let priority = workInProgress.pendingUpdatePriority;
     if (workInProgress.pendingProps.hidden && priority !== OffscreenPriority) {
       // If this host component is hidden, we can reconcile its children at
       // the lowest priority and bail out from this particular pass. Unless, we're
@@ -310,7 +325,7 @@ module.exports = function<T, P, I, C>(config : HostConfig<T, P, I, C>, getSchedu
     workInProgress.memoizedProps = workInProgress.pendingProps;
     workInProgress.memoizedState = state;
     workInProgress.output = current.output;
-    const priorityLevel = workInProgress.pendingWorkPriority;
+    const priorityLevel = workInProgress.pendingUpdatePriority;
     workInProgress.pendingProps = null;
     workInProgress.updateQueue = current.updateQueue = null;
     workInProgress.stateNode = current.stateNode;
@@ -324,7 +339,7 @@ module.exports = function<T, P, I, C>(config : HostConfig<T, P, I, C>, getSchedu
       if (workInProgress.pendingWorkPriority !== NoWork && workInProgress.pendingWorkPriority <= priorityLevel) {
         return findNextUnitOfWorkAtPriority(
           workInProgress,
-          workInProgress.pendingWorkPriority
+          priorityLevel
         );
       } else {
         return null;
@@ -339,7 +354,7 @@ module.exports = function<T, P, I, C>(config : HostConfig<T, P, I, C>, getSchedu
     // If we started this work before, and finished it, or if we're in a
     // ping-pong update scenario, this version could already be what we're
     // looking for. In that case, we should be able to just bail out.
-    const priorityLevel = workInProgress.pendingWorkPriority;
+    const priorityLevel = workInProgress.pendingUpdatePriority;
     workInProgress.pendingProps = null;
     workInProgress.updateQueue = null;
     if (workInProgress.alternate) {

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/ReactFiberPendingWork.js

@@ -27,7 +27,7 @@ function cloneSiblings(current : Fiber, workInProgress : Fiber, returnFiber : Fi
     current = current.sibling;
     workInProgress = workInProgress.sibling = cloneFiber(
       current,
-      current.pendingWorkPriority
+      current.pendingUpdatePriority
     );
     workInProgress.return = returnFiber;
   }
@@ -66,7 +66,7 @@ function cloneChildrenIfNeeded(workInProgress : Fiber) {
   }
   workInProgress.child = cloneFiber(
     currentChild,
-    currentChild.pendingWorkPriority
+    currentChild.pendingUpdatePriority
   );
   cloneSiblings(currentChild, workInProgress.child, workInProgress);
 }
@@ -77,7 +77,9 @@ exports.findNextUnitOfWorkAtPriority = function(workRoot : Fiber, priorityLevel
     if (workInProgress.pendingWorkPriority !== NoWork &&
         workInProgress.pendingWorkPriority <= priorityLevel) {
       // This node has work to do that fits our priority level criteria.
-      if (workInProgress.pendingProps !== null || workInProgress.updateQueue !== null) {
+      if (workInProgress.pendingUpdatePriority !== NoWork &&
+          workInProgress.pendingUpdatePriority <= priorityLevel &&
+          (workInProgress.pendingProps !== null || workInProgress.updateQueue !== null)) {
         return workInProgress;
       }
 
@@ -94,14 +96,9 @@ exports.findNextUnitOfWorkAtPriority = function(workRoot : Fiber, priorityLevel
         }
         child = workInProgress.childInProgress;
         while (child) {
-          // Don't bother drilling further down this tree if there is no child
-          // with more content.
-          // TODO: Shouldn't this still drill down even though the first
-          // shallow level doesn't have anything pending on it.
-          if (child.pendingWorkPriority !== NoWork &&
-              child.pendingWorkPriority <= priorityLevel &&
-              child.pendingProps !== null) {
-            return child;
+          const next = exports.findNextUnitOfWorkAtPriority(child, priorityLevel);
+          if (next) {
+            return next;
           }
           child = child.sibling;
         }
@@ -113,13 +110,15 @@ exports.findNextUnitOfWorkAtPriority = function(workRoot : Fiber, priorityLevel
       // If we match the priority but has no child and no work to do,
       // then we can safely reset the flag.
       workInProgress.pendingWorkPriority = NoWork;
+      workInProgress.pendingUpdatePriority = NoWork;
     }
     if (workInProgress === workRoot) {
       if (workInProgress.pendingWorkPriority <= priorityLevel) {
         // If this subtree had work left to do, we would have returned it by
         // now. This could happen if a child with pending work gets cleaned up
         // but we don't clear the flag then. It is safe to reset it now.
         workInProgress.pendingWorkPriority = NoWork;
+        workInProgress.pendingUpdatePriority = NoWork;
       }
       return null;
     }
@@ -133,6 +132,7 @@ exports.findNextUnitOfWorkAtPriority = function(workRoot : Fiber, priorityLevel
         // now. This could happen if a child with pending work gets cleaned up
         // but we don't clear the flag then. It is safe to reset it now.
         workInProgress.pendingWorkPriority = NoWork;
+        workInProgress.pendingUpdatePriority = NoWork;
       }
     }
     workInProgress.sibling.return = workInProgress.return;

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

@@ -78,8 +78,9 @@ module.exports = function<T, P, I, C>(config : HostConfig<T, P, I, C>) {
     while (root) {
       let rootInProgress = cloneFiber(
         root.current,
-        root.current.pendingWorkPriority
+        root.current.pendingUpdatePriority
       );
+
       // Find the highest possible priority work to do.
       // This loop is unrolled just to satisfy Flow's enum constraint.
       // We could make arbitrary many idle priority levels but having
@@ -151,6 +152,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({