fix: improve performance of dashboard replay

project-codeflare · Apr 9, 2023 · 2388228 · 2388228
1 parent 0bf9ead
commit 2388228
Show file tree

Hide file tree

Showing 3 changed files with 44 additions and 27 deletions.
diff --git a/plugins/plugin-codeflare-dashboard/src/components/Dashboard/index.tsx b/plugins/plugin-codeflare-dashboard/src/components/Dashboard/index.tsx
@@ -80,23 +80,41 @@ export default class Dashboard extends React.PureComponent<Props, State> {
     }
   }
 
-  /** We have received an update from `this.props.initWatcher` */
+  /** Helps with debouncing updates of WorkersUpdate */
+  private updateDebounce1: (null | ReturnType<typeof setTimeout>)[] = []
+
+  /** Helps with debouncing updates of LogLineUpdate */
+  private updateDebounce2: (null | ReturnType<typeof setTimeout>)[] = []
+
+  /** We have received an update from a `GridSpec.initWatcher` */
   private readonly onUpdate = (gridIdx: number, model: UpdatePayload) => {
-    this.setState((curState) => ({
-      firstUpdate: (curState && curState.firstUpdate) || Date.now(), // TODO pull from the events
-      lastUpdate: Date.now(), // TODO pull from the events
-      events: !isWorkersUpdate(model)
-        ? curState?.events
-        : !model.events || model.events.length === 0
-        ? curState?.events
-        : model.events,
-      logLine: !isWorkersUpdate(model) ? model.logLine : curState?.logLine,
-      workers: !isWorkersUpdate(model)
-        ? curState?.workers
-        : !curState?.workers
-        ? [model.workers]
-        : [...curState.workers.slice(0, gridIdx), model.workers, ...curState.workers.slice(gridIdx + 1)],
-    }))
+    // to avoid a flurry of renders, we do some debouncing here
+    const bouncies = isWorkersUpdate(model) ? this.updateDebounce1 : this.updateDebounce2
+    const bouncey = bouncies[gridIdx]
+    if (bouncey !== null) clearTimeout(bouncey)
+
+    bouncies[gridIdx] = setTimeout(
+      () =>
+        this.setState((curState) => ({
+          lastUpdate: Date.now(), // TODO pull from the events
+          firstUpdate: (curState && curState.firstUpdate) || Date.now(), // TODO pull from the events
+
+          logLine: !isWorkersUpdate(model) ? model.logLine : curState?.logLine,
+
+          events: !isWorkersUpdate(model)
+            ? curState?.events
+            : !model.events || model.events.length === 0
+            ? curState?.events
+            : model.events,
+
+          workers: !isWorkersUpdate(model)
+            ? curState?.workers
+            : !curState?.workers
+            ? [model.workers]
+            : [...curState.workers.slice(0, gridIdx), model.workers, ...curState.workers.slice(gridIdx + 1)],
+        })),
+      0
+    )
   }
 
   /** @return the grid models, excluding the `null` linebreak indicators */

diff --git a/plugins/plugin-codeflare-dashboard/src/controller/dashboard/status/Live.ts b/plugins/plugin-codeflare-dashboard/src/controller/dashboard/status/Live.ts
@@ -181,16 +181,12 @@ export default class Live {
     }
   }
 
-  /** Helps with debouncing logLine updates */
-  private logLineTO: null | ReturnType<typeof setTimeout> = null
-
   /** Add the given `line` to our logLines model and pass the updated model to `cb` */
   private pushLineAndPublish(logLine: string, cb: OnData) {
     if (logLine) {
       // here we avoid a flood of React renders by batching them up a
       // bit; i thought react 18 was supposed to help with this. hmm.
-      if (this.logLineTO) clearTimeout(this.logLineTO)
-      this.logLineTO = setTimeout(() => cb({ logLine }), 1)
+      cb({ logLine })
     }
   }
 

diff --git a/plugins/plugin-codeflare-dashboard/src/controller/dashboard/utilization/Live.ts b/plugins/plugin-codeflare-dashboard/src/controller/dashboard/utilization/Live.ts
@@ -51,7 +51,10 @@ export default class Live {
   ) {
     tails.map((tailf) => {
       tailf.then(({ stream }) => {
-        stream.on("data", (data) => {
+        // async here to improve interleaving (i.e. the async is used
+        // to introduce a thread yield), not because we have anything
+        // inherently asynchronous to do
+        stream.on("data", async (data) => {
           if (data) {
             const line = stripAnsi(data)
             const cols = line.split(/\s+/)
@@ -115,11 +118,6 @@ export default class Live {
                   // out of date event, drop it
                   return
                 }
-
-                // inform the UI that we have updates
-                cb({
-                  workers: Object.values(this.workers),
-                })
               }
 
               if (name === "*") {
@@ -129,6 +127,11 @@ export default class Live {
                 // this event affects a specific worker
                 update(name)
               }
+
+              // inform the UI that we have updates
+              cb({
+                workers: Object.values(this.workers),
+              })
             }
           }
         })