From d4e971a26606e44e9dcd862512b9a9eece350e14 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Mon, 9 Jan 2017 14:13:48 -0800 Subject: [PATCH 1/2] Remove callbackList field from Fiber Moves it to UpdateQueue instead so that we don't waste memory for components that don't have update queues. --- src/renderers/shared/fiber/ReactFiber.js | 4 +- .../shared/fiber/ReactFiberCommitWork.js | 12 ++--- src/renderers/shared/fiber/ReactFiberRoot.js | 1 - .../shared/fiber/ReactFiberUpdateQueue.js | 47 ++++++++----------- 4 files changed, 26 insertions(+), 38 deletions(-) diff --git a/src/renderers/shared/fiber/ReactFiber.js b/src/renderers/shared/fiber/ReactFiber.js index ea0b88bb60e06..9cd295fc5c693 100644 --- a/src/renderers/shared/fiber/ReactFiber.js +++ b/src/renderers/shared/fiber/ReactFiber.js @@ -111,8 +111,7 @@ export type Fiber = { // A queue of state updates and callbacks. updateQueue: UpdateQueue | null, - // A list of callbacks that should be called during the next commit. - callbackList: UpdateQueue | null, + // The state used to create the output memoizedState: any, @@ -203,7 +202,6 @@ var createFiber = function(tag : TypeOfWork, key : null | string) : Fiber { pendingProps: null, memoizedProps: null, updateQueue: null, - callbackList: null, memoizedState: null, effectTag: NoEffect, diff --git a/src/renderers/shared/fiber/ReactFiberCommitWork.js b/src/renderers/shared/fiber/ReactFiberCommitWork.js index 382046ff24e47..c7abfc390c564 100644 --- a/src/renderers/shared/fiber/ReactFiberCommitWork.js +++ b/src/renderers/shared/fiber/ReactFiberCommitWork.js @@ -30,6 +30,7 @@ var { commitCallbacks } = require('ReactFiberUpdateQueue'); var { Placement, Update, + Callback, ContentReset, } = require('ReactTypeOfSideEffect'); @@ -410,17 +411,16 @@ module.exports = function( } } } - const callbackList = finishedWork.callbackList; - if (callbackList) { - commitCallbacks(finishedWork, callbackList, instance); + if ((finishedWork.effectTag & Callback) && finishedWork.updateQueue) { + commitCallbacks(finishedWork, finishedWork.updateQueue, instance); } return; } case HostRoot: { - const callbackList = finishedWork.callbackList; - if (callbackList) { + const updateQueue = finishedWork.updateQueue; + if (updateQueue) { const instance = finishedWork.child && finishedWork.child.stateNode; - commitCallbacks(finishedWork, callbackList, instance); + commitCallbacks(finishedWork, updateQueue, instance); } return; } diff --git a/src/renderers/shared/fiber/ReactFiberRoot.js b/src/renderers/shared/fiber/ReactFiberRoot.js index 5d60c608fd054..3bb55ff52d978 100644 --- a/src/renderers/shared/fiber/ReactFiberRoot.js +++ b/src/renderers/shared/fiber/ReactFiberRoot.js @@ -39,7 +39,6 @@ exports.createFiberRoot = function(containerInfo : any) : FiberRoot { containerInfo: containerInfo, isScheduled: false, nextScheduledRoot: null, - callbackList: null, context: null, pendingContext: null, }; diff --git a/src/renderers/shared/fiber/ReactFiberUpdateQueue.js b/src/renderers/shared/fiber/ReactFiberUpdateQueue.js index 72fdeb4e06178..e12ca9175613f 100644 --- a/src/renderers/shared/fiber/ReactFiberUpdateQueue.js +++ b/src/renderers/shared/fiber/ReactFiberUpdateQueue.js @@ -59,6 +59,7 @@ export type UpdateQueue = { first: Update | null, last: Update | null, hasForceUpdate: boolean, + callbackList: null | Array, // Dev only isProcessing?: boolean, @@ -95,6 +96,7 @@ function ensureUpdateQueue(fiber : Fiber) : UpdateQueue { first: null, last: null, hasForceUpdate: false, + callbackList: null, isProcessing: false, }; } else { @@ -102,6 +104,7 @@ function ensureUpdateQueue(fiber : Fiber) : UpdateQueue { first: null, last: null, hasForceUpdate: false, + callbackList: null, }; } @@ -122,6 +125,8 @@ function cloneUpdateQueue(alt : Fiber, fiber : Fiber) : UpdateQueue | null { altQueue.first = sourceQueue.first; altQueue.last = sourceQueue.last; altQueue.hasForceUpdate = sourceQueue.hasForceUpdate; + altQueue.callbackList = sourceQueue.callbackList; + altQueue.isProcessing = sourceQueue.isProcessing; alt.updateQueue = altQueue; return altQueue; } @@ -438,29 +443,20 @@ function beginUpdateQueue( // Second condition ignores top-level unmount callbacks if they are not the // last update in the queue, since a subsequent update will cause a remount. if (update.callback && !(update.isTopLevelUnmount && update.next)) { - const callbackUpdate = cloneUpdate(update); - if (callbackList && callbackList.last) { - callbackList.last.next = callbackUpdate; - callbackList.last = callbackUpdate; - } else { - callbackList = { - first: callbackUpdate, - last: callbackUpdate, - hasForceUpdate: false, - }; - } + callbackList = callbackList || []; + callbackList.push(update.callback); workInProgress.effectTag |= CallbackEffect; } update = update.next; } - if (!queue.first && !queue.hasForceUpdate) { - // Queue is now empty + queue.callbackList = callbackList; + + if (!queue.first && !callbackList && !queue.hasForceUpdate) { + // The queue is empty and there are no callbacks. We can reset it. workInProgress.updateQueue = null; } - workInProgress.callbackList = callbackList; - if (__DEV__) { queue.isProcessing = false; } @@ -469,19 +465,14 @@ function beginUpdateQueue( } exports.beginUpdateQueue = beginUpdateQueue; -function commitCallbacks(finishedWork : Fiber, callbackList : UpdateQueue, context : mixed) { - const stopAfter = callbackList.last; - let update = callbackList.first; - while (update) { - const callback = update.callback; - if (typeof callback === 'function') { - callback.call(context); - } - if (update === stopAfter) { - break; - } - update = update.next; +function commitCallbacks(finishedWork : Fiber, queue : UpdateQueue, context : mixed) { + const callbackList = queue.callbackList; + if (!callbackList) { + return; + } + for (let i = 0; i < callbackList.length; i++) { + const callback = callbackList[i]; + callback.call(context); } - finishedWork.callbackList = null; } exports.commitCallbacks = commitCallbacks; From 288a4c4d6db841da0afe99d057fddf151c339ddb Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Mon, 9 Jan 2017 17:14:09 -0800 Subject: [PATCH 2/2] Reset invalid UpdateQueue fields when cloning from current Similar to what cloneFiber does. No change in behavior, but it's more consistent this way. --- src/renderers/shared/fiber/ReactFiber.js | 2 +- .../shared/fiber/ReactFiberUpdateQueue.js | 26 +++++++++++-------- 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/src/renderers/shared/fiber/ReactFiber.js b/src/renderers/shared/fiber/ReactFiber.js index 9cd295fc5c693..d3f3a282d22e7 100644 --- a/src/renderers/shared/fiber/ReactFiber.js +++ b/src/renderers/shared/fiber/ReactFiber.js @@ -277,7 +277,7 @@ exports.cloneFiber = function(fiber : Fiber, priorityLevel : PriorityLevel) : Fi // pendingProps is here for symmetry but is unnecessary in practice for now. // TODO: Pass in the new pendingProps as an argument maybe? alt.pendingProps = fiber.pendingProps; - cloneUpdateQueue(alt, fiber); + cloneUpdateQueue(fiber, alt); alt.pendingWorkPriority = priorityLevel; alt.memoizedProps = fiber.memoizedProps; diff --git a/src/renderers/shared/fiber/ReactFiberUpdateQueue.js b/src/renderers/shared/fiber/ReactFiberUpdateQueue.js index e12ca9175613f..bb70bec77d20a 100644 --- a/src/renderers/shared/fiber/ReactFiberUpdateQueue.js +++ b/src/renderers/shared/fiber/ReactFiberUpdateQueue.js @@ -113,21 +113,25 @@ function ensureUpdateQueue(fiber : Fiber) : UpdateQueue { } // Clones an update queue from a source fiber onto its alternate. -function cloneUpdateQueue(alt : Fiber, fiber : Fiber) : UpdateQueue | null { - const sourceQueue = fiber.updateQueue; - if (!sourceQueue) { +function cloneUpdateQueue(current : Fiber, workInProgress : Fiber) : UpdateQueue | null { + const currentQueue = current.updateQueue; + if (!currentQueue) { // The source fiber does not have an update queue. - alt.updateQueue = null; + workInProgress.updateQueue = null; return null; } // If the alternate already has a queue, reuse the previous object. - const altQueue = alt.updateQueue || {}; - altQueue.first = sourceQueue.first; - altQueue.last = sourceQueue.last; - altQueue.hasForceUpdate = sourceQueue.hasForceUpdate; - altQueue.callbackList = sourceQueue.callbackList; - altQueue.isProcessing = sourceQueue.isProcessing; - alt.updateQueue = altQueue; + const altQueue = workInProgress.updateQueue || {}; + altQueue.first = currentQueue.first; + altQueue.last = currentQueue.last; + + // These fields are invalid by the time we clone from current. Reset them. + altQueue.hasForceUpdate = false; + altQueue.callbackList = null; + altQueue.isProcessing = false; + + workInProgress.updateQueue = altQueue; + return altQueue; } exports.cloneUpdateQueue = cloneUpdateQueue;