perf(sessions): make opening a big task feel fast#3075
Conversation
|
React Doctor found no issues in the changed files. 🎉 Reviewed by React Doctor for commit |
|
Reviews (1): Last reviewed commit: "perf(sessions): seek real content in the..." | Re-trigger Greptile |
| do { | ||
| const res = (await windowQuery.query({ | ||
| taskRunId, | ||
| endOffset, | ||
| maxBytes: OPEN_TAIL_BYTES, | ||
| })) as { | ||
| content: string; | ||
| startOffset: number; | ||
| endOffset: number; | ||
| headReached: boolean; | ||
| } | null; | ||
| if (!res) break; | ||
| const { rawEntries } = this.parseLogContent(res.content); | ||
| events = [...convertStoredEntriesToEvents(rawEntries), ...events]; | ||
| endOffset = res.startOffset; | ||
| headReached = res.headReached; | ||
| bytesRead += res.endOffset - res.startOffset; | ||
| } while ( | ||
| !headReached && | ||
| bytesRead < OPEN_TAIL_MAX_BYTES && | ||
| !this.tailHasRenderableContent(events) | ||
| ); |
There was a problem hiding this comment.
Infinite loop when a single ndjson line exceeds
maxBytes
When the tail of the log contains a line longer than OPEN_TAIL_BYTES (1.5 MB — a realistic size for a tool call embedding a large file), readLocalLogsWindow returns startOffset ≈ endOffset (no newline is found within the window, so the entire raw string is "dropped" and the retained-content boundary lands at the end of the window). The caller then sets endOffset = res.startOffset for the next iteration, which equals the value it just passed in, so every subsequent query reads the exact same bytes and produces the same result. Since bytesRead += res.endOffset - res.startOffset adds 0 each round, the bytesRead < OPEN_TAIL_MAX_BYTES guard never fires, and the loop runs indefinitely.
Fix: detect the no-progress case and break early. After updating bytesRead, check if (res.endOffset <= res.startOffset) break; — or equivalently check whether endOffset changed — before re-evaluating the while condition.
The pool self-terminates when the last diff view unmounts, so opening a diff-heavy task after navigating away re-pays the worker's one-time init (WASM regex engine + theme normalization, ~1.7-2.2s). Mount one pool provider at the app root so that init happens once at startup and every diff render reuses the warm worker. Measured on a 20MB diff-heavy task: time-to-content 3364ms -> 1902ms (the ~2.2s of off-main Shiki init drops out of the open path entirely).
The tail-first paint read only the last 1.5MB, but a heavily-resumed task appends megabytes of config/mode/resume blobs there, so the paint showed an empty transcript and the user waited for the full read+build+render. Replace readLocalLogsTail with a byte-windowed readLocalLogsWindow and page back past the noise until real messages are in view, then paint them; the full read still loads the complete transcript behind it. Measured on a heavily-resumed 20MB task: time-to-first-content 1902ms -> 1066ms (and ~150ms for a typical task whose tail already holds messages).
7d1606c to
58e5936
Compare
|
Closing — verified benchmarks show no measurable open-speed win (equal warm, slightly slower cold due to serial config-noise paging), and the warm diff-worker pool would pin ~8 Shiki workers in memory for the whole session. The task-open speedup already shipped in #3063. Details in the thread. |
Problem
Opening a task with a large transcript felt slow — up to ~3.4s before anything showed in the task log, and hover/input froze during it. Two independent causes, both scaling with transcript size:
config_option_update/current_mode_update/ resume blobs to the end, so the paint landed on pure noise, showed an empty transcript, and the user waited for the full read + build + render.Changes
WorkerPoolContextProviderat the app root so the pool's mount count never hits zero. Shiki inits once at startup; every subsequent diff render reuses the warm worker.readLocalLogsTailwith a byte-windowedreadLocalLogsWindowand page back past the config-noise until real messages are in view, then paint them. The full read still loads the complete transcript behind the paint.Both are contained; no behavior change for small tasks (their tail already holds messages). Does not touch the separate ~1s of synchronous main-thread work (parse/build/render) — that's a bigger, off-main-parsing change left for later.
How did you test this?
readLocalLogsWindow(whole-file, tail-with-partial-line-drop, line-boundary, single-line-over-max, backwards paging reconstructs the file, missing log);paintTailFirst(paints when the tail holds content, pages back past config-noise, no-ops for missing host read / existing session / empty content).Automatic notifications