Skip to content

Commit c55b626

Browse files
committed
Use queueMicrotask to batch bridge messages
This timeout is too long. It makes the UI feels sluggish. Chrome also throttles looping timers aggressively which makes it worse. We also shouldn't rely on the throttling at this level. It doesn't help when you spam the receiving side with thousands of messages to process anyway. Instead we need to implement a form of backpressure to avoid sending so much in the first place.
1 parent 8ee7311 commit c55b626

File tree

2 files changed

+29
-26
lines changed

2 files changed

+29
-26
lines changed

packages/react-devtools-shared/src/__tests__/setupTests.js

+5
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,11 @@ beforeEach(() => {
128128
// Fake timers let us flush Bridge operations between setup and assertions.
129129
jest.useFakeTimers();
130130

131+
// We use fake timers heavily in tests but the bridge batching now uses microtasks.
132+
global.devtoolsJestTestScheduler = callback => {
133+
setTimeout(callback, 0);
134+
};
135+
131136
// Use utils.js#withErrorsOrWarningsIgnored instead of directly mutating this array.
132137
global._ignoredErrorOrWarningMessages = [
133138
'react-test-renderer is deprecated.',

packages/react-devtools-shared/src/bridge.js

+24-26
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,6 @@ import type {
1919
} from 'react-devtools-shared/src/backend/types';
2020
import type {StyleAndLayout as StyleAndLayoutPayload} from 'react-devtools-shared/src/backend/NativeStyleEditor/types';
2121

22-
const BATCH_DURATION = 100;
23-
2422
// This message specifies the version of the DevTools protocol currently supported by the backend,
2523
// as well as the earliest NPM version (e.g. "4.13.0") that protocol is supported by on the frontend.
2624
// This enables an older frontend to display an upgrade message to users for a newer, unsupported backend.
@@ -276,7 +274,7 @@ class Bridge<
276274
}> {
277275
_isShutdown: boolean = false;
278276
_messageQueue: Array<any> = [];
279-
_timeoutID: TimeoutID | null = null;
277+
_scheduledFlush: boolean = false;
280278
_wall: Wall;
281279
_wallUnlisten: Function | null = null;
282280

@@ -324,8 +322,19 @@ class Bridge<
324322
// (or we're waiting for our setTimeout-0 to fire), then _timeoutID will
325323
// be set, and we'll simply add to the queue and wait for that
326324
this._messageQueue.push(event, payload);
327-
if (!this._timeoutID) {
328-
this._timeoutID = setTimeout(this._flush, 0);
325+
if (!this._scheduledFlush) {
326+
this._scheduledFlush = true;
327+
// $FlowFixMe
328+
if (typeof devtoolsJestTestScheduler === 'function') {
329+
// This exists just for our own jest tests.
330+
// They're written in such a way that we can neither mock queueMicrotask
331+
// because then we break React DOM and we can't not mock it because then
332+
// we can't synchronously flush it. So they need to be rewritten.
333+
// $FlowFixMe
334+
devtoolsJestTestScheduler(this._flush); // eslint-disable-line no-undef
335+
} else {
336+
queueMicrotask(this._flush);
337+
}
329338
}
330339
}
331340

@@ -363,34 +372,23 @@ class Bridge<
363372
do {
364373
this._flush();
365374
} while (this._messageQueue.length);
366-
367-
// Make sure once again that there is no dangling timer.
368-
if (this._timeoutID !== null) {
369-
clearTimeout(this._timeoutID);
370-
this._timeoutID = null;
371-
}
372375
}
373376

374377
_flush: () => void = () => {
375378
// This method is used after the bridge is marked as destroyed in shutdown sequence,
376379
// so we do not bail out if the bridge marked as destroyed.
377380
// It is a private method that the bridge ensures is only called at the right times.
378-
379-
if (this._timeoutID !== null) {
380-
clearTimeout(this._timeoutID);
381-
this._timeoutID = null;
382-
}
383-
384-
if (this._messageQueue.length) {
385-
for (let i = 0; i < this._messageQueue.length; i += 2) {
386-
this._wall.send(this._messageQueue[i], ...this._messageQueue[i + 1]);
381+
try {
382+
if (this._messageQueue.length) {
383+
for (let i = 0; i < this._messageQueue.length; i += 2) {
384+
this._wall.send(this._messageQueue[i], ...this._messageQueue[i + 1]);
385+
}
386+
this._messageQueue.length = 0;
387387
}
388-
this._messageQueue.length = 0;
389-
390-
// Check again for queued messages in BATCH_DURATION ms. This will keep
391-
// flushing in a loop as long as messages continue to be added. Once no
392-
// more are, the timer expires.
393-
this._timeoutID = setTimeout(this._flush, BATCH_DURATION);
388+
} finally {
389+
// We set this at the end in case new messages are added synchronously above.
390+
// They're already handled so they shouldn't queue more flushes.
391+
this._scheduledFlush = false;
394392
}
395393
};
396394

0 commit comments

Comments
 (0)