perf(desktop): Codex 等のフル再描画 TUI 実行時の重さを軽減 (#293)#299
Conversation
Codex などフル画面再描画型の TUI を走らせるとターミナルが重くなる 問題への対応。daemon 側 Session で以下 2 点を導入: - broadcast コアレス: data イベントを 16ms or 128KB ごとに結合して 1 回のブロードキャストに束ねる。JSON.stringify と socket.write、 renderer 側 xterm.write の呼び出し回数を削減 - emulator write コアレス: キュー内の連続した小チャンクを MAX_CHUNK_CHARS まで結合してから emulator.write を 1 回呼ぶ。 ANSI パーサのセットアップ回数を削減 順序保証のため exit/error イベント直前、attach 時、dispose 時は 必ず flush。snapshot boundary の item 単位カウントは保持。 SUPERSET_TERMINAL_BROADCAST_COALESCE=0 および SUPERSET_TERMINAL_EMULATOR_COALESCE=0 で個別に無効化可能。
📝 WalkthroughWalkthroughターミナルセッションの処理において、PTYデータイベントと エミュレータ書き込み操作をそれぞれバッファリングして定期的にまとめて処理する仕組みを追加。ブロードキャストコアレッシングとエミュレータコアレッシングの2つの最適化を実装しました。 Changes
Sequence Diagram(s)sequenceDiagram
participant PTY as PTY Data
participant Queue as Broadcast Queue
participant Emulator as Emulator
participant Client as Client
rect rgba(100, 150, 200, 0.5)
Note over PTY,Client: 従来フロー：即座にブロードキャスト
PTY->>Client: broadcastEvent("data")
end
rect rgba(150, 200, 100, 0.5)
Note over PTY,Client: 新フロー：データコアレッシング
PTY->>Queue: queueBroadcastData()
Note over Queue: バッファリング
Client->>Queue: attach() / non-data event
Queue->>Client: flushPendingBroadcastData()
Queue->>Client: broadcastEvent("data") x N [まとめて]
end
sequenceDiagram
participant Queue as Emulator Queue
participant Processor as processEmulatorWriteQueue()
participant Emulator as emulator.write()
rect rgba(200, 100, 150, 0.5)
Note over Queue,Emulator: エミュレータコアレッシング（フラグON時）
Queue->>Processor: [chunk1, chunk2, chunk3, ...]
Processor->>Processor: 連続チャンクをマージ
Processor->>Emulator: emulator.write(merged_data)
Note over Emulator: 単一呼び出しで処理
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0cf80ca49f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| this.enqueueEmulatorWrite(this.scanState.heldBytes); | ||
| this.broadcastEvent("data", { | ||
| type: "data", | ||
| data: this.scanState.heldBytes, | ||
| } satisfies TerminalDataEvent); | ||
| this.queueBroadcastData(this.scanState.heldBytes); | ||
| this.scanState.heldBytes = ""; |
There was a problem hiding this comment.
When a session exits before shell readiness completes, handleSubprocessExit() calls resolveShellReady("timed_out") and then resetProcessState(). In this block, held scanner bytes are now sent via queueBroadcastData, which defers emission behind the coalescing timer; resetProcessState() then clears the pending coalesce buffer, so those final bytes are never delivered to attached clients. This is a regression from the previous immediate broadcastEvent behavior and can drop terminal output for short-lived shells/commands that terminate during startup marker scanning.
Useful? React with 👍 / 👎.
resolveShellReady may enqueue held scanner bytes via queueBroadcastData during subprocess exit. resetProcessState previously cleared the coalesce buffer without flushing, silently dropping those final bytes for short-lived shells that terminate during startup marker scanning. Replace the clear with a flush so the data reaches attached clients before teardown completes. Matches the pre-coalescing behavior. Addresses Codex review on #299.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/desktop/src/main/terminal-host/session.ts (1)
655-711: emulator write coalesce のロジックは正しく、emulatorWriteProcessedItemsの会計も一致しています。内側の while 条件
merged.length + queue[0].length <= MAX_CHUNK_CHARSと、その手前の oversized head 分岐（Line 664）の組み合わせにより、FIFO 順序・サロゲートペア境界・スナップショット境界カウント（itemsConsumed相当分の加算）のいずれも pre-coalesce 時の振る舞いと等価に保たれています。itemsConsumed === 0の防衛ブレーク（Line 699）は、oversized 分岐が先に処理される以上、実際には到達不能ですが念のための防御として残す意図は理解できます。細かい点ですが、Line 694 の
this.emulatorWriteQueue.shift() as stringは、直前の条件でqueue.length > 0が保証されているので non-null 断言（!）の方が意図が明確かもしれません。必須ではありません。♻️ 任意のリファクタ案
- const nextChunk = this.emulatorWriteQueue.shift() as string; + const nextChunk = this.emulatorWriteQueue.shift()!;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/main/terminal-host/session.ts` around lines 655 - 711, The review notes that inside the coalescing loop the call this.emulatorWriteQueue.shift() as string can be made clearer by using a non-null assertion because the while condition already guarantees queue length > 0; replace that cast with this.emulatorWriteQueue.shift()! (in the block handling merged/itemsConsumed) to express the guaranteed non-null value while leaving the defensive itemsConsumed === 0 check intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/desktop/src/main/terminal-host/session.ts`:
- Around line 655-711: The review notes that inside the coalescing loop the call
this.emulatorWriteQueue.shift() as string can be made clearer by using a
non-null assertion because the while condition already guarantees queue length >
0; replace that cast with this.emulatorWriteQueue.shift()! (in the block
handling merged/itemsConsumed) to express the guaranteed non-null value while
leaving the defensive itemsConsumed === 0 check intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7cd2c121-ff82-440f-befa-9a8fedbcacf4
📒 Files selected for processing (1)
apps/desktop/src/main/terminal-host/session.ts
1 participant
Summary
Issue #293 対応。Codex のようなフル画面再描画型 TUI をターミナルで動かすとアプリが重くなる問題に対し、daemon 側
Sessionでコアレス処理を 2 段導入しました。背景と調査
apps/desktop/src/main/terminal-host/session.tsの data イベント経路で 2 つのボトルネックを確認：JSON.stringify()+socket.write()が走るprocessEmulatorWriteQueue()が小チャンクを 1 件ずつemulator.write()に渡し、ANSI パーサのセットアップが何度も発生xterm.writeも呼び出し回数に比例して重くなる変更点
apps/desktop/src/main/terminal-host/session.tsのみ変更。Phase 1: broadcast data coalesce
BROADCAST_COALESCE_INTERVAL_MS = 16(ms) またはBROADCAST_COALESCE_MAX_BYTES = 131_072に達するまで内部バッファに蓄え、1 回のブロードキャストに束ねるexit/errorイベントを送る直前で強制 flush（順序保証）attach()で既存クライアントに対して pending を flush してから新クライアントを追加（snapshot との二重送信回避）dispose()/resetProcessState()でバッファを解放Phase 2: emulator write coalesce
processEmulatorWriteQueue()で連続する小チャンクをMAX_CHUNK_CHARS(8192) まで結合してからemulator.write()を 1 回呼ぶemulatorWriteProcessedItemsは item 単位で加算し続けるためflushToSnapshotBoundary()の境界計算は変更なし期待効果と副作用
テスト計画
bun test src/main/terminal-host/全 37 件パスtsc --noEmitで本差分由来の型エラーなしbiome checkクリアSUPERSET_TERMINAL_BROADCAST_COALESCE=0/SUPERSET_TERMINAL_EMULATOR_COALESCE=0での無効化切り戻し確認Closes #293
Summary by CodeRabbit