@samitouri / QOS-React / commits / 61777a78f6

[scheduler] 3/n Use a linked list instead of map and queue for callback storage (#12893)

* [schedule] Use linked list instead of queue and map for storing cbs NOTE: This PR depends on https://github.com/facebook/react/pull/12880 and https://github.com/facebook/react/pull/12884 Please review those first, and after they land Flarnie will rebase on top of them. --- **what is the change?:** See title **why make this change?:** This seems to make the code simpler, and potentially saves space of having an array and object around holding references to the callbacks. **test plan:** Run existing tests * minor style improvements * refactor conditionals in cancelScheduledWork for increased clarity * Remove 'canUseDOM' condition and fix some flow issues w/callbackID type **what is the change?:** - Removed conditional which fell back to 'setTimeout' when the environment doesn't have DOM. This appears to be an old polyfill used for test environments and we don't use it any more. - Fixed type definitions around the callbackID to be more accurate in the scheduler itself, and more loose in the React code. **why make this change?:** To get Flow passing, simplify the scheduler code, make things accurate. **test plan:** Run tests and flow. * Rewrite 'cancelScheduledWork' so that Flow accepts it **what is the change?:** Adding verification that 'previousCallbackConfig' and 'nextCallbackConfig' are not null before accessing properties on them. Slightly concerned because this implementation relies on these properties being untouched and correct on the config which is passed to 'cancelScheduledWork' but I guess we already rely heavily on that for this whole approach. :\ **why make this change?:** To get Flow passing. Not sure why it passed earlier and in CI, but now it's not. **test plan:** `yarn flow dom` and other flow tests, lint, tests, etc. * ran prettier * Put back the fallback implementation of scheduler for node environment **what is the change?:** We had tried removing the fallback implementation of `scheduler` but tests reminded us that this is important for supporting isomorphic uses of React. Long term we will move this out of the `schedule` module but for now let's keep things simple. **why make this change?:** Keep things working! **test plan:** Ran tests and flow * Shorten properties stored in objects by sheduler **what is the change?:** `previousScheduledCallback` -> `prev` `nextScheduledCallback` -> `next` **why make this change?:** We want this package to be smaller, and less letters means less code means smaller! **test plan:** ran existing tests * further remove extra lines in scheduler

Flarnie Marchan committed May 26, 2018 at 15:55 UTC 61777a78f6f9d90bea6ecdab4038675010bd70db
3 files changed +134 -77
packages/react-reconciler/src/ReactFiberScheduler.js
+7 -5
@@ -1430,7 +1430,7 @@ let firstScheduledRoot: FiberRoot | null = null;
1430 let lastScheduledRoot: FiberRoot | null = null;
1431
1432 let callbackExpirationTime: ExpirationTime = NoWork;
1433 -let callbackID: number = -1;
1433 +let callbackID: *;
1434 let isRendering: boolean = false;
1435 let nextFlushedRoot: FiberRoot | null = null;
1436 let nextFlushedExpirationTime: ExpirationTime = NoWork;
@@ -1459,9 +1459,11 @@ function scheduleCallbackWithExpiration(expirationTime) {
1459 // Existing callback has sufficient timeout. Exit.
1460 return;
1461 } else {
1462 - // Existing callback has insufficient timeout. Cancel and schedule a
1463 - // new one.
1464 - cancelDeferredCallback(callbackID);
1462 + if (callbackID !== null) {
1463 + // Existing callback has insufficient timeout. Cancel and schedule a
1464 + // new one.
1465 + cancelDeferredCallback(callbackID);
1466 + }
1467 }
1468 // The request callback timer is already running. Don't start a new one.
1469 } else {
@@ -1687,7 +1689,7 @@ function performWork(
1689 // If we're inside a callback, set this to false since we just completed it.
1690 if (deadline !== null) {
1691 callbackExpirationTime = NoWork;
1690 - callbackID = -1;
1692 + callbackID = null;
1693 }
1694 // If there's work left over, schedule a new callback.
1695 if (nextFlushedExpirationTime !== NoWork) {
packages/react-scheduler/src/ReactScheduler.js
+125 -71
@@ -35,11 +35,14 @@ type FrameCallbackType = Deadline => void;
35 type CallbackConfigType = {|
36 scheduledCallback: FrameCallbackType,
37 timeoutTime: number,
38 - callbackId: number, // used for cancelling
38 + next: CallbackConfigType | null, // creating a linked list
39 + prev: CallbackConfigType | null, // creating a linked list
40 |};
41
41 -import ExecutionEnvironment from 'fbjs/lib/ExecutionEnvironment';
42 +export type CallbackIdType = CallbackConfigType;
43 +
44 import requestAnimationFrameForReact from 'shared/requestAnimationFrameForReact';
45 +import ExecutionEnvironment from 'fbjs/lib/ExecutionEnvironment';
46
47 const hasNativePerformanceNow =
48 typeof performance === 'object' && typeof performance.now === 'function';
@@ -55,24 +58,26 @@ if (hasNativePerformanceNow) {
58 };
59 }
60
58 -// TODO: There's no way to cancel, because Fiber doesn't atm.
61 let scheduleWork: (
62 callback: FrameCallbackType,
63 options?: {timeout: number},
62 -) => number;
63 -let cancelScheduledWork: (callbackId: number) => void;
64 +) => CallbackIdType;
65 +let cancelScheduledWork: (callbackId: CallbackIdType) => void;
66
67 if (!ExecutionEnvironment.canUseDOM) {
66 - let callbackIdCounter = 0;
67 - // Timeouts are objects in Node.
68 - // For consistency, we'll use numbers in the public API anyway.
69 - const timeoutIds: {[number]: TimeoutID} = {};
68 + const timeoutIds = new Map();
69
70 scheduleWork = function(
71 callback: FrameCallbackType,
72 options?: {timeout: number},
74 - ): number {
75 - const callbackId = callbackIdCounter++;
73 + ): CallbackIdType {
74 + // keeping return type consistent
75 + const callbackConfig = {
76 + scheduledCallback: callback,
77 + timeoutTime: 0,
78 + next: null,
79 + prev: null,
80 + };
81 const timeoutId = setTimeout(() => {
82 callback({
83 timeRemaining() {
@@ -81,34 +86,18 @@ if (!ExecutionEnvironment.canUseDOM) {
86 didTimeout: false,
87 });
88 });
84 - timeoutIds[callbackId] = timeoutId;
85 - return callbackId;
89 + timeoutIds.set(callback, timeoutId);
90 + return callbackConfig;
91 };
87 - cancelScheduledWork = function(callbackId: number) {
88 - const timeoutId = timeoutIds[callbackId];
89 - delete timeoutIds[callbackId];
92 + cancelScheduledWork = function(callbackId: CallbackIdType) {
93 + const callback = callbackId.scheduledCallback;
94 + const timeoutId = timeoutIds.get(callback);
95 + timeoutIds.delete(callbackId);
96 clearTimeout(timeoutId);
97 };
98 } else {
93 - // We keep callbacks in a queue.
94 - // Calling scheduleWork 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 -
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 'cancelScheduledWork' 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} = {};
99 + let headOfPendingCallbacksLinkedList: CallbackConfigType | null = null;
100 + let tailOfPendingCallbacksLinkedList: CallbackConfigType | null = null;
101
102 // We track what the next soonest timeoutTime is, to be able to quickly tell
103 // if none of the scheduled callbacks have timed out.
@@ -132,30 +121,13 @@ if (!ExecutionEnvironment.canUseDOM) {
121 },
122 };
123
135 - const safelyCallScheduledCallback = function(
136 - callback: FrameCallbackType,
137 - callbackId: number,
138 - ) {
139 - if (!registeredCallbackIds[callbackId]) {
140 - // ignore cancelled callbacks
141 - return;
142 - }
143 - try {
144 - callback(frameDeadlineObject);
145 - // Avoid using 'catch' to keep errors easy to debug
146 - } finally {
147 - // always clean up the callbackId, even if the callback throws
148 - delete registeredCallbackIds[callbackId];
149 - }
150 - };
151 -
124 /**
125 * Checks for timed out callbacks, runs them, and then checks again to see if
126 * any more have timed out.
127 * Keeps doing this until there are none which have currently timed out.
128 */
129 const callTimedOutCallbacks = function() {
158 - if (pendingCallbacks.length === 0) {
130 + if (headOfPendingCallbacksLinkedList === null) {
131 return;
132 }
133
@@ -176,14 +148,17 @@ if (!ExecutionEnvironment.canUseDOM) {
148
149 // keep checking until we don't find any more timed out callbacks
150 frameDeadlineObject.didTimeout = true;
179 - for (let i = 0, len = pendingCallbacks.length; i < len; i++) {
180 - const currentCallbackConfig = pendingCallbacks[i];
151 + let currentCallbackConfig = headOfPendingCallbacksLinkedList;
152 + while (currentCallbackConfig !== null) {
153 const timeoutTime = currentCallbackConfig.timeoutTime;
154 if (timeoutTime !== -1 && timeoutTime <= currentTime) {
155 // it has timed out!
156 // call it
157 const callback = currentCallbackConfig.scheduledCallback;
186 - safelyCallScheduledCallback(callback, currentCallbackConfig.callbackId);
158 + // TODO: error handling
159 + callback(frameDeadlineObject);
160 + // remove it from linked list
161 + cancelScheduledWork(currentCallbackConfig);
162 } else {
163 if (
164 timeoutTime !== -1 &&
@@ -193,6 +168,7 @@ if (!ExecutionEnvironment.canUseDOM) {
168 nextSoonestTimeoutTime = timeoutTime;
169 }
170 }
171 + currentCallbackConfig = currentCallbackConfig.next;
172 }
173 };
174
@@ -208,7 +184,7 @@ if (!ExecutionEnvironment.canUseDOM) {
184 }
185 isIdleScheduled = false;
186
211 - if (pendingCallbacks.length === 0) {
187 + if (headOfPendingCallbacksLinkedList === null) {
188 return;
189 }
190
@@ -217,15 +193,28 @@ if (!ExecutionEnvironment.canUseDOM) {
193
194 let currentTime = now();
195 // Next, as long as we have idle time, try calling more callbacks.
220 - while (frameDeadline - currentTime > 0 && pendingCallbacks.length > 0) {
221 - const latestCallbackConfig = pendingCallbacks.shift();
196 + while (
197 + frameDeadline - currentTime > 0 &&
198 + headOfPendingCallbacksLinkedList !== null
199 + ) {
200 + const latestCallbackConfig = headOfPendingCallbacksLinkedList;
201 + // move head of list to next callback
202 + headOfPendingCallbacksLinkedList = latestCallbackConfig.next;
203 + if (headOfPendingCallbacksLinkedList !== null) {
204 + headOfPendingCallbacksLinkedList.prev = null;
205 + } else {
206 + // if headOfPendingCallbacksLinkedList is null,
207 + // then the list must be empty.
208 + // make sure we set the tail to null as well.
209 + tailOfPendingCallbacksLinkedList = null;
210 + }
211 frameDeadlineObject.didTimeout = false;
212 const latestCallback = latestCallbackConfig.scheduledCallback;
224 - const newCallbackId = latestCallbackConfig.callbackId;
225 - safelyCallScheduledCallback(latestCallback, newCallbackId);
213 + // TODO: before using this outside of React we need to add error handling
214 + latestCallback(frameDeadlineObject);
215 currentTime = now();
216 }
228 - if (pendingCallbacks.length > 0) {
217 + if (headOfPendingCallbacksLinkedList !== null) {
218 if (!isAnimationFrameScheduled) {
219 // Schedule another animation callback so we retry later.
220 isAnimationFrameScheduled = true;
@@ -271,7 +260,7 @@ if (!ExecutionEnvironment.canUseDOM) {
260 scheduleWork = function(
261 callback: FrameCallbackType,
262 options?: {timeout: number},
274 - ): number {
263 + ): CallbackIdType /* CallbackConfigType */ {
264 let timeoutTime = -1;
265 if (options != null && typeof options.timeout === 'number') {
266 timeoutTime = now() + options.timeout;
@@ -283,15 +272,27 @@ if (!ExecutionEnvironment.canUseDOM) {
272 nextSoonestTimeoutTime = timeoutTime;
273 }
274
286 - const newCallbackId = getCallbackId();
287 - const scheduledCallbackConfig = {
275 + const scheduledCallbackConfig: CallbackConfigType = {
276 scheduledCallback: callback,
289 - callbackId: newCallbackId,
277 timeoutTime,
278 + prev: null,
279 + next: null,
280 };
292 - pendingCallbacks.push(scheduledCallbackConfig);
281 + if (headOfPendingCallbacksLinkedList === null) {
282 + // Make this callback the head and tail of our list
283 + headOfPendingCallbacksLinkedList = scheduledCallbackConfig;
284 + tailOfPendingCallbacksLinkedList = scheduledCallbackConfig;
285 + } else {
286 + // Add latest callback as the new tail of the list
287 + scheduledCallbackConfig.prev = tailOfPendingCallbacksLinkedList;
288 + // renaming for clarity
289 + const oldTailOfPendingCallbacksLinkedList = tailOfPendingCallbacksLinkedList;
290 + if (oldTailOfPendingCallbacksLinkedList !== null) {
291 + oldTailOfPendingCallbacksLinkedList.next = scheduledCallbackConfig;
292 + }
293 + tailOfPendingCallbacksLinkedList = scheduledCallbackConfig;
294 + }
295
294 - registeredCallbackIds[newCallbackId] = true;
296 if (!isAnimationFrameScheduled) {
297 // If rAF didn't already schedule one, we need to schedule a frame.
298 // TODO: If this rAF doesn't materialize because the browser throttles, we
@@ -300,11 +301,64 @@ if (!ExecutionEnvironment.canUseDOM) {
301 isAnimationFrameScheduled = true;
302 requestAnimationFrameForReact(animationTick);
303 }
303 - return newCallbackId;
304 + return scheduledCallbackConfig;
305 };
306
306 - cancelScheduledWork = function(callbackId: number) {
307 - delete registeredCallbackIds[callbackId];
307 + cancelScheduledWork = function(
308 + callbackConfig: CallbackIdType /* CallbackConfigType */,
309 + ) {
310 + /**
311 + * There are four possible cases:
312 + * - Head/nodeToRemove/Tail -> null
313 + * In this case we set Head and Tail to null.
314 + * - Head -> ... middle nodes... -> Tail/nodeToRemove
315 + * In this case we point the middle.next to null and put middle as the new
316 + * Tail.
317 + * - Head/nodeToRemove -> ...middle nodes... -> Tail
318 + * In this case we point the middle.prev at null and move the Head to
319 + * middle.
320 + * - Head -> ... ?some nodes ... -> nodeToRemove -> ... ?some nodes ... -> Tail
321 + * In this case we point the Head.next to the Tail and the Tail.prev to
322 + * the Head.
323 + */
324 + const next = callbackConfig.next;
325 + const prev = callbackConfig.prev;
326 + if (next !== null) {
327 + // we have a next
328 +
329 + if (prev !== null) {
330 + // we have a prev
331 +
332 + // callbackConfig is somewhere in the middle of a list of 3 or more nodes.
333 + prev.next = next;
334 + next.prev = prev;
335 + return;
336 + } else {
337 + // there is a next but not a previous one;
338 + // callbackConfig is the head of a list of 2 or more other nodes.
339 + next.prev = null;
340 + headOfPendingCallbacksLinkedList = next;
341 + return;
342 + }
343 + } else {
344 + // there is no next callback config; this must the tail of the list
345 +
346 + if (prev !== null) {
347 + // we have a prev
348 +
349 + // callbackConfig is the tail of a list of 2 or more other nodes.
350 + prev.next = null;
351 + tailOfPendingCallbacksLinkedList = prev;
352 + return;
353 + } else {
354 + // there is no previous callback config;
355 + // callbackConfig is the only thing in the linked list,
356 + // so both head and tail point to it.
357 + headOfPendingCallbacksLinkedList = null;
358 + tailOfPendingCallbacksLinkedList = null;
359 + return;
360 + }
361 + }
362 };
363 }
364
packages/react-test-renderer/src/ReactTestRendererScheduling.js
+2 -1
@@ -19,7 +19,8 @@ export function scheduleDeferredCallback(
19 options?: {timeout: number},
20 ): number {
21 scheduledCallback = callback;
22 - return 0;
22 + const fakeCallbackId = 0;
23 + return fakeCallbackId;
24 }
25
26 export function cancelDeferredCallback(timeoutID: number): void {