From c55b626df39dd6bd14397219661c1ffb209ac29b Mon Sep 17 00:00:00 2001
From: Sebastian Markbage <sebastian@calyptus.eu>
Date: Wed, 31 Jul 2024 20:55:05 -0400
Subject: [PATCH] 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.
---
 .../src/__tests__/setupTests.js               |  5 ++
 packages/react-devtools-shared/src/bridge.js  | 50 +++++++++----------
 2 files changed, 29 insertions(+), 26 deletions(-)

diff --git a/packages/react-devtools-shared/src/__tests__/setupTests.js b/packages/react-devtools-shared/src/__tests__/setupTests.js
index b78a3d162629c..79cda67f99b18 100644
--- a/packages/react-devtools-shared/src/__tests__/setupTests.js
+++ b/packages/react-devtools-shared/src/__tests__/setupTests.js
@@ -128,6 +128,11 @@ beforeEach(() => {
   // Fake timers let us flush Bridge operations between setup and assertions.
   jest.useFakeTimers();
 
+  // We use fake timers heavily in tests but the bridge batching now uses microtasks.
+  global.devtoolsJestTestScheduler = callback => {
+    setTimeout(callback, 0);
+  };
+
   // Use utils.js#withErrorsOrWarningsIgnored instead of directly mutating this array.
   global._ignoredErrorOrWarningMessages = [
     'react-test-renderer is deprecated.',
diff --git a/packages/react-devtools-shared/src/bridge.js b/packages/react-devtools-shared/src/bridge.js
index f8d418f1005ba..f4da08be6bdbe 100644
--- a/packages/react-devtools-shared/src/bridge.js
+++ b/packages/react-devtools-shared/src/bridge.js
@@ -19,8 +19,6 @@ import type {
 } from 'react-devtools-shared/src/backend/types';
 import type {StyleAndLayout as StyleAndLayoutPayload} from 'react-devtools-shared/src/backend/NativeStyleEditor/types';
 
-const BATCH_DURATION = 100;
-
 // This message specifies the version of the DevTools protocol currently supported by the backend,
 // as well as the earliest NPM version (e.g. "4.13.0") that protocol is supported by on the frontend.
 // This enables an older frontend to display an upgrade message to users for a newer, unsupported backend.
@@ -276,7 +274,7 @@ class Bridge<
 }> {
   _isShutdown: boolean = false;
   _messageQueue: Array<any> = [];
-  _timeoutID: TimeoutID | null = null;
+  _scheduledFlush: boolean = false;
   _wall: Wall;
   _wallUnlisten: Function | null = null;
 
@@ -324,8 +322,19 @@ class Bridge<
     //   (or we're waiting for our setTimeout-0 to fire), then _timeoutID will
     //   be set, and we'll simply add to the queue and wait for that
     this._messageQueue.push(event, payload);
-    if (!this._timeoutID) {
-      this._timeoutID = setTimeout(this._flush, 0);
+    if (!this._scheduledFlush) {
+      this._scheduledFlush = true;
+      // $FlowFixMe
+      if (typeof devtoolsJestTestScheduler === 'function') {
+        // This exists just for our own jest tests.
+        // They're written in such a way that we can neither mock queueMicrotask
+        // because then we break React DOM and we can't not mock it because then
+        // we can't synchronously flush it. So they need to be rewritten.
+        // $FlowFixMe
+        devtoolsJestTestScheduler(this._flush); // eslint-disable-line no-undef
+      } else {
+        queueMicrotask(this._flush);
+      }
     }
   }
 
@@ -363,34 +372,23 @@ class Bridge<
     do {
       this._flush();
     } while (this._messageQueue.length);
-
-    // Make sure once again that there is no dangling timer.
-    if (this._timeoutID !== null) {
-      clearTimeout(this._timeoutID);
-      this._timeoutID = null;
-    }
   }
 
   _flush: () => void = () => {
     // This method is used after the bridge is marked as destroyed in shutdown sequence,
     // so we do not bail out if the bridge marked as destroyed.
     // It is a private method that the bridge ensures is only called at the right times.
-
-    if (this._timeoutID !== null) {
-      clearTimeout(this._timeoutID);
-      this._timeoutID = null;
-    }
-
-    if (this._messageQueue.length) {
-      for (let i = 0; i < this._messageQueue.length; i += 2) {
-        this._wall.send(this._messageQueue[i], ...this._messageQueue[i + 1]);
+    try {
+      if (this._messageQueue.length) {
+        for (let i = 0; i < this._messageQueue.length; i += 2) {
+          this._wall.send(this._messageQueue[i], ...this._messageQueue[i + 1]);
+        }
+        this._messageQueue.length = 0;
       }
-      this._messageQueue.length = 0;
-
-      // Check again for queued messages in BATCH_DURATION ms. This will keep
-      // flushing in a loop as long as messages continue to be added. Once no
-      // more are, the timer expires.
-      this._timeoutID = setTimeout(this._flush, BATCH_DURATION);
+    } finally {
+      // We set this at the end in case new messages are added synchronously above.
+      // They're already handled so they shouldn't queue more flushes.
+      this._scheduledFlush = false;
     }
   };