@samitouri / QOS-React / commits / a9abd27e4f

[schedule] Support multiple callbacks in scheduler (#12746)

* Support using id to cancel scheduled callback **what is the change?:** see title **why make this change?:** Once we support multiple callbacks you will need to use the id to specify which callback you mean. **test plan:** Added a test, ran all tests, lint, etc. * ran prettier * fix lint * Use object for storing callback info in scheduler * Wrap initial test in a describe block * Support multiple callbacks in `ReactScheduler` **what is the change?:** We keep a queue of callbacks instead of just one at a time, and call them in order first by their timeoutTime and then by the order which they were scheduled in. **why make this change?:** We plan on using this module to coordinate JS outside of React, so we will need to schedule more than one callback at a time. **test plan:** Added a boatload of shiny new tests. :) Plus ran all the old ones. NOTE: The tests do not yet cover the vital logic of callbacks timing out, and later commits will add the missing test coverage. * Heuristic to avoid looking for timed out callbacks when none timed out **what is the change?:** Tracks the current soonest timeOut time for all scheduled callbacks. **why make this change?:** We were checking every scheduled callback to see if it timed out on every tick. It's more efficient to skip that O(n) check if we know that none have timed out. **test plan:** Ran existing tests. Will write new tests to cover timeout behavior in more detail soon. * Put multiple callback support under a disabled feature flag **what is the change?:** See title **why make this change?:** We don't have error handling in place yet, so should maintain the old behavior until that is in place. But want to get this far to continue making incremental changes. **test plan:** Updated and ran tests. * Hide support for multiple callbacks under a feature flag **what is the change?:** see title **why make this change?:** We haven't added error handling yet, so should not expose this feature. **test plan:** Ran all tests, temporarily split out the tests for multiple callbacks into separate file. Will recombine once we remove the flag. * Fix nits from code review See comments on https://github.com/facebook/react/pull/12743 * update checklist in comments * Remove nested loop which calls additional timed out callbacks **what is the change?:** We used to re-run any callbacks which time out whilst other callbacks are running, but now we will only check once for timed out callbacks then then run them. **why make this change?:** To simplify the code and the behavior of this module. **test plan:** Ran all existing tests. * Remove feature flag **what is the change?:** see title **why make this change?:** Because only React is using this, and it sounds like async. rendering won't hit any different behavior due to these changes. **test plan:** Existing tests pass, and this allowed us to recombine all tests to run in both 'test' and 'test-build' modes. * remove outdated file * fix typo

Flarnie Marchan committed May 9, 2018 at 15:28 UTC a9abd27e4f5aa1e68bd6035be901299327279ee2
3 files changed +292 -53
packages/react-scheduler/src/ReactScheduler.js
+129 -41
@@ -14,8 +14,8 @@
14 * control than requestAnimationFrame and requestIdleCallback.
15 * Current TODO items:
16 * X- Pull out the rIC polyfill built into React
17 - * - Initial test coverage
18 - * - Support for multiple callbacks
17 + * X- Initial test coverage
18 + * X- Support for multiple callbacks
19 * - Support for two priorities; serial and deferred
20 * - Better test coverage
21 * - Better docblock
@@ -31,6 +31,11 @@
31 // The frame rate is dynamically adjusted.
32
33 import type {Deadline} from 'react-reconciler';
34 +type CallbackConfigType = {|
35 + scheduledCallback: Deadline => void,
36 + timeoutTime: number,
37 + callbackId: number, // used for cancelling
38 +|};
39
40 import ExecutionEnvironment from 'fbjs/lib/ExecutionEnvironment';
41 import warning from 'fbjs/lib/warning';
@@ -85,12 +90,31 @@ if (!ExecutionEnvironment.canUseDOM) {
90 clearTimeout(timeoutID);
91 };
92 } else {
88 - // Always polyfill requestIdleCallback and cancelIdleCallback
93 + // We keep callbacks in a queue.
94 + // Calling rIC will push in a new callback at the end of the queue.
95 + // When we get idle time, callbacks are removed from the front of the queue
96 + // and called.
97 + const pendingCallbacks: Array<CallbackConfigType> = [];
98
90 - let scheduledRICCallback = null;
91 - let isIdleScheduled = false;
92 - let timeoutTime = -1;
99 + let callbackIdCounter = 0;
100 + const getCallbackId = function(): number {
101 + callbackIdCounter++;
102 + return callbackIdCounter;
103 + };
104 +
105 + // When a callback is scheduled, we register it by adding it's id to this
106 + // object.
107 + // If the user calls 'cIC' with the id of that callback, it will be
108 + // unregistered by removing the id from this object.
109 + // Then we skip calling any callback which is not registered.
110 + // This means cancelling is an O(1) time complexity instead of O(n).
111 + const registeredCallbackIds: {[number]: boolean} = {};
112
113 + // We track what the next soonest timeoutTime is, to be able to quickly tell
114 + // if none of the scheduled callbacks have timed out.
115 + let nextSoonestTimeoutTime = -1;
116 +
117 + let isIdleScheduled = false;
118 let isAnimationFrameScheduled = false;
119
120 let frameDeadline = 0;
@@ -100,7 +124,7 @@ if (!ExecutionEnvironment.canUseDOM) {
124 let previousFrameTime = 33;
125 let activeFrameTime = 33;
126
103 - const frameDeadlineObject = {
127 + const frameDeadlineObject: Deadline = {
128 didTimeout: false,
129 timeRemaining() {
130 const remaining = frameDeadline - now();
@@ -108,6 +132,67 @@ if (!ExecutionEnvironment.canUseDOM) {
132 },
133 };
134
135 + const safelyCallScheduledCallback = function(callback, callbackId) {
136 + if (!registeredCallbackIds[callbackId]) {
137 + // ignore cancelled callbacks
138 + return;
139 + }
140 + try {
141 + callback(frameDeadlineObject);
142 + // Avoid using 'catch' to keep errors easy to debug
143 + } finally {
144 + // always clean up the callbackId, even if the callback throws
145 + delete registeredCallbackIds[callbackId];
146 + }
147 + };
148 +
149 + /**
150 + * Checks for timed out callbacks, runs them, and then checks again to see if
151 + * any more have timed out.
152 + * Keeps doing this until there are none which have currently timed out.
153 + */
154 + const callTimedOutCallbacks = function() {
155 + if (pendingCallbacks.length === 0) {
156 + return;
157 + }
158 +
159 + const currentTime = now();
160 + // TODO: this would be more efficient if deferred callbacks are stored in
161 + // min heap.
162 + // Or in a linked list with links for both timeoutTime order and insertion
163 + // order.
164 + // For now an easy compromise is the current approach:
165 + // Keep a pointer to the soonest timeoutTime, and check that first.
166 + // If it has not expired, we can skip traversing the whole list.
167 + // If it has expired, then we step through all the callbacks.
168 + if (nextSoonestTimeoutTime === -1 || nextSoonestTimeoutTime > currentTime) {
169 + // We know that none of them have timed out yet.
170 + return;
171 + }
172 + nextSoonestTimeoutTime = -1; // we will reset it below
173 +
174 + // keep checking until we don't find any more timed out callbacks
175 + frameDeadlineObject.didTimeout = true;
176 + for (let i = 0, len = pendingCallbacks.length; i < len; i++) {
177 + const currentCallbackConfig = pendingCallbacks[i];
178 + const timeoutTime = currentCallbackConfig.timeoutTime;
179 + if (timeoutTime !== -1 && timeoutTime <= currentTime) {
180 + // it has timed out!
181 + // call it
182 + const callback = currentCallbackConfig.scheduledCallback;
183 + safelyCallScheduledCallback(callback, timeoutTime);
184 + } else {
185 + if (
186 + timeoutTime !== -1 &&
187 + (nextSoonestTimeoutTime === -1 ||
188 + timeoutTime < nextSoonestTimeoutTime)
189 + ) {
190 + nextSoonestTimeoutTime = timeoutTime;
191 + }
192 + }
193 + }
194 + };
195 +
196 // We use the postMessage trick to defer idle work until after the repaint.
197 const messageKey =
198 '__reactIdleCallback$' +
@@ -119,36 +204,30 @@ if (!ExecutionEnvironment.canUseDOM) {
204 return;
205 }
206
207 + if (pendingCallbacks.length === 0) {
208 + return;
209 + }
210 isIdleScheduled = false;
211
124 - const currentTime = now();
125 - if (frameDeadline - currentTime <= 0) {
126 - // There's no time left in this idle period. Check if the callback has
127 - // a timeout and whether it's been exceeded.
128 - if (timeoutTime !== -1 && timeoutTime <= currentTime) {
129 - // Exceeded the timeout. Invoke the callback even though there's no
130 - // time left.
131 - frameDeadlineObject.didTimeout = true;
132 - } else {
133 - // No timeout.
134 - if (!isAnimationFrameScheduled) {
135 - // Schedule another animation callback so we retry later.
136 - isAnimationFrameScheduled = true;
137 - requestAnimationFrame(animationTick);
138 - }
139 - // Exit without invoking the callback.
140 - return;
141 - }
142 - } else {
143 - // There's still time left in this idle period.
212 + // First call anything which has timed out, until we have caught up.
213 + callTimedOutCallbacks();
214 +
215 + let currentTime = now();
216 + // Next, as long as we have idle time, try calling more callbacks.
217 + while (frameDeadline - currentTime > 0 && pendingCallbacks.length > 0) {
218 + const latestCallbackConfig = pendingCallbacks.shift();
219 frameDeadlineObject.didTimeout = false;
220 + const latestCallback = latestCallbackConfig.scheduledCallback;
221 + const newCallbackId = latestCallbackConfig.callbackId;
222 + safelyCallScheduledCallback(latestCallback, newCallbackId);
223 + currentTime = now();
224 }
146 -
147 - timeoutTime = -1;
148 - const callback = scheduledRICCallback;
149 - scheduledRICCallback = null;
150 - if (callback !== null) {
151 - callback(frameDeadlineObject);
225 + if (pendingCallbacks.length > 0) {
226 + if (!isAnimationFrameScheduled) {
227 + // Schedule another animation callback so we retry later.
228 + isAnimationFrameScheduled = true;
229 + requestAnimationFrame(animationTick);
230 + }
231 }
232 };
233 // Assumes that we have addEventListener in this environment. Might need
@@ -190,12 +269,23 @@ if (!ExecutionEnvironment.canUseDOM) {
269 callback: (deadline: Deadline) => void,
270 options?: {timeout: number},
271 ): number {
193 - // This assumes that we only schedule one callback at a time because that's
194 - // how Fiber uses it.
195 - scheduledRICCallback = callback;
272 + let timeoutTime = -1;
273 if (options != null && typeof options.timeout === 'number') {
274 timeoutTime = now() + options.timeout;
275 }
276 + if (timeoutTime > nextSoonestTimeoutTime) {
277 + nextSoonestTimeoutTime = timeoutTime;
278 + }
279 +
280 + const newCallbackId = getCallbackId();
281 + const scheduledCallbackConfig = {
282 + scheduledCallback: callback,
283 + callbackId: newCallbackId,
284 + timeoutTime,
285 + };
286 + pendingCallbacks.push(scheduledCallbackConfig);
287 +
288 + registeredCallbackIds[newCallbackId] = true;
289 if (!isAnimationFrameScheduled) {
290 // If rAF didn't already schedule one, we need to schedule a frame.
291 // TODO: If this rAF doesn't materialize because the browser throttles, we
@@ -204,13 +294,11 @@ if (!ExecutionEnvironment.canUseDOM) {
294 isAnimationFrameScheduled = true;
295 requestAnimationFrame(animationTick);
296 }
207 - return 0;
297 + return newCallbackId;
298 };
299
210 - cIC = function() {
211 - scheduledRICCallback = null;
212 - isIdleScheduled = false;
213 - timeoutTime = -1;
300 + cIC = function(callbackId: number) {
301 + delete registeredCallbackIds[callbackId];
302 };
303 }
304
packages/react-scheduler/src/__tests__/ReactScheduler-test.js
+161 -10
@@ -41,16 +41,167 @@ describe('ReactScheduler', () => {
41 ReactScheduler = require('react-scheduler');
42 });
43
44 - it('rIC calls the callback within the frame when not blocked', () => {
45 - const {rIC} = ReactScheduler;
46 - const cb = jest.fn();
47 - rIC(cb);
48 - jest.runAllTimers();
49 - expect(cb.mock.calls.length).toBe(1);
50 - // should have ... TODO details on what we expect
51 - expect(cb.mock.calls[0][0].didTimeout).toBe(false);
52 - expect(typeof cb.mock.calls[0][0].timeRemaining()).toBe('number');
44 + describe('rIC', () => {
45 + it('calls the callback within the frame when not blocked', () => {
46 + const {rIC} = ReactScheduler;
47 + const cb = jest.fn();
48 + rIC(cb);
49 + jest.runAllTimers();
50 + expect(cb.mock.calls.length).toBe(1);
51 + // should not have timed out and should include a timeRemaining method
52 + expect(cb.mock.calls[0][0].didTimeout).toBe(false);
53 + expect(typeof cb.mock.calls[0][0].timeRemaining()).toBe('number');
54 + });
55 +
56 + describe('with multiple callbacks', () => {
57 + it('accepts multiple callbacks and calls within frame when not blocked', () => {
58 + const {rIC} = ReactScheduler;
59 + const callbackLog = [];
60 + const callbackA = jest.fn(() => callbackLog.push('A'));
61 + const callbackB = jest.fn(() => callbackLog.push('B'));
62 + rIC(callbackA);
63 + // initially waits to call the callback
64 + expect(callbackLog).toEqual([]);
65 + // waits while second callback is passed
66 + rIC(callbackB);
67 + expect(callbackLog).toEqual([]);
68 + // after a delay, calls as many callbacks as it has time for
69 + jest.runAllTimers();
70 + expect(callbackLog).toEqual(['A', 'B']);
71 + // callbackA should not have timed out and should include a timeRemaining method
72 + expect(callbackA.mock.calls[0][0].didTimeout).toBe(false);
73 + expect(typeof callbackA.mock.calls[0][0].timeRemaining()).toBe(
74 + 'number',
75 + );
76 + // callbackA should not have timed out and should include a timeRemaining method
77 + expect(callbackB.mock.calls[0][0].didTimeout).toBe(false);
78 + expect(typeof callbackB.mock.calls[0][0].timeRemaining()).toBe(
79 + 'number',
80 + );
81 + });
82 +
83 + it(
84 + 'schedules callbacks in correct order and' +
85 + 'keeps calling them if there is time',
86 + () => {
87 + const {rIC} = ReactScheduler;
88 + const callbackLog = [];
89 + const callbackA = jest.fn(() => {
90 + callbackLog.push('A');
91 + rIC(callbackC);
92 + });
93 + const callbackB = jest.fn(() => {
94 + callbackLog.push('B');
95 + });
96 + const callbackC = jest.fn(() => {
97 + callbackLog.push('C');
98 + });
99 +
100 + rIC(callbackA);
101 + // initially waits to call the callback
102 + expect(callbackLog).toEqual([]);
103 + // continues waiting while B is scheduled
104 + rIC(callbackB);
105 + expect(callbackLog).toEqual([]);
106 + // after a delay, calls the scheduled callbacks,
107 + // and also calls new callbacks scheduled by current callbacks
108 + jest.runAllTimers();
109 + expect(callbackLog).toEqual(['A', 'B', 'C']);
110 + },
111 + );
112 +
113 + it('schedules callbacks in correct order when callbacks have many nested rIC calls', () => {
114 + const {rIC} = ReactScheduler;
115 + const callbackLog = [];
116 + const callbackA = jest.fn(() => {
117 + callbackLog.push('A');
118 + rIC(callbackC);
119 + rIC(callbackD);
120 + });
121 + const callbackB = jest.fn(() => {
122 + callbackLog.push('B');
123 + rIC(callbackE);
124 + rIC(callbackF);
125 + });
126 + const callbackC = jest.fn(() => {
127 + callbackLog.push('C');
128 + });
129 + const callbackD = jest.fn(() => {
130 + callbackLog.push('D');
131 + });
132 + const callbackE = jest.fn(() => {
133 + callbackLog.push('E');
134 + });
135 + const callbackF = jest.fn(() => {
136 + callbackLog.push('F');
137 + });
138 +
139 + rIC(callbackA);
140 + rIC(callbackB);
141 + // initially waits to call the callback
142 + expect(callbackLog).toEqual([]);
143 + // while flushing callbacks, calls as many as it has time for
144 + jest.runAllTimers();
145 + expect(callbackLog).toEqual(['A', 'B', 'C', 'D', 'E', 'F']);
146 + });
147 +
148 + it('schedules callbacks in correct order when they use rIC to schedule themselves', () => {
149 + const {rIC} = ReactScheduler;
150 + const callbackLog = [];
151 + let callbackAIterations = 0;
152 + const callbackA = jest.fn(() => {
153 + if (callbackAIterations < 1) {
154 + rIC(callbackA);
155 + }
156 + callbackLog.push('A' + callbackAIterations);
157 + callbackAIterations++;
158 + });
159 + const callbackB = jest.fn(() => callbackLog.push('B'));
160 +
161 + rIC(callbackA);
162 + // initially waits to call the callback
163 + expect(callbackLog).toEqual([]);
164 + rIC(callbackB);
165 + expect(callbackLog).toEqual([]);
166 + // after a delay, calls the latest callback passed
167 + jest.runAllTimers();
168 + expect(callbackLog).toEqual(['A0', 'B', 'A1']);
169 + });
170 + });
171 + });
172 +
173 + describe('cIC', () => {
174 + it('cancels the scheduled callback', () => {
175 + const {rIC, cIC} = ReactScheduler;
176 + const cb = jest.fn();
177 + const callbackId = rIC(cb);
178 + expect(cb.mock.calls.length).toBe(0);
179 + cIC(callbackId);
180 + jest.runAllTimers();
181 + expect(cb.mock.calls.length).toBe(0);
182 + });
183 +
184 + describe('with multiple callbacks', () => {
185 + it('when one callback cancels the next one', () => {
186 + const {rIC, cIC} = ReactScheduler;
187 + const callbackLog = [];
188 + let callbackBId;
189 + const callbackA = jest.fn(() => {
190 + callbackLog.push('A');
191 + cIC(callbackBId);
192 + });
193 + const callbackB = jest.fn(() => callbackLog.push('B'));
194 + rIC(callbackA);
195 + callbackBId = rIC(callbackB);
196 + // Initially doesn't call anything
197 + expect(callbackLog).toEqual([]);
198 + jest.runAllTimers();
199 + // B should not get called because A cancelled B
200 + expect(callbackLog).toEqual(['A']);
201 + expect(callbackB.mock.calls.length).toBe(0);
202 + });
203 + });
204 });
205
55 - // TODO: test cIC and now
206 + // TODO: test 'now'
207 });
packages/shared/forks/ReactFeatureFlags.www.js
+2 -2
@@ -12,11 +12,11 @@ import typeof * as FeatureFlagsShimType from './ReactFeatureFlags.www';
12
13 // Re-export dynamic flags from the www version.
14 export const {
15 - enableGetDerivedStateFromCatch,
15 debugRenderPhaseSideEffects,
16 debugRenderPhaseSideEffectsForStrictMode,
18 - warnAboutDeprecatedLifecycles,
17 + enableGetDerivedStateFromCatch,
18 replayFailedUnitOfWorkWithInvokeGuardedCallback,
19 + warnAboutDeprecatedLifecycles,
20 } = require('ReactFeatureFlags');
21
22 // The rest of the flags are static for better dead code elimination.