Conversation
📝 WalkthroughWalkthroughエミュレータの書き込みキューの条件付き遅延ドレインを導入し、アンマウント時の端末イベントをバッファリングしてフォーカス/接続状態に基づくGPU有効化を追加。WebGL初期化をオンデマンド化し、タイマーと破棄のライフサイクルを管理する変更を加えた。 Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Session as TerminalHostSession
participant Emulator
participant Snapshot
Client->>Session: 接続/切断・入力送信
Session->>Emulator: writeQueue にデータ追加
alt 条件付きアイドルドレイン (no client / ready / below high-water / no waiters)
Session->>Session: schedule idle drain (250ms)
Session-->>Emulator: バッチで drain (idle)
else 緊急ドレイン (snapshot boundary / flush waiter / urgent)
Session->>Emulator: immediate urgent drain
Session->>Snapshot: スナップショット境界処理
end
Emulator-->>Client: 出力配信（接続時）
Note over Session,Emulator: emulatorIdleDrainTimer のライフサイクル管理（クリア/再スケジュール）
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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 |
# Conflicts: # apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/v1-terminal-cache.ts
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
apps/desktop/src/main/terminal-host/session.ts (1)
1102-1136:resetProcessStateでアイドルドレインタイマーがクリアされない
dispose()(Line 1073) ではclearIdleEmulatorDrainTimer()を呼んでいますが、handleSubprocessExit経由で呼ばれるresetProcessState()ではクリアされていません。サブプロセスが終了してemulatorWriteQueueをクリアした後でも、最大 250ms 後にタイマーが発火し、空キューに対するsetImmediate→processEmulatorWriteQueueが走ります。動作としては無害ですが、本 PR の趣旨（アイドル時の不要な処理を止める）と矛盾するため、dispose()と対称にクリアしておくのが自然です。♻️ 提案する修正
private resetProcessState(): void { this.subprocess = null; this.subprocessReady = false; this.subprocessDecoder = null; @@ this.emulatorWriteQueue = []; this.emulatorWriteQueuedBytes = 0; this.emulatorWriteProcessedItems = 0; this.nextSnapshotBoundaryWaiterId = 1; this.emulatorWriteScheduled = false; + this.clearIdleEmulatorDrainTimer(); this.resolveAllSnapshotBoundaryWaiters();🤖 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 1102 - 1136, resetProcessState currently doesn't clear the idle emulator drain timer, so after a subprocess exit a pending idle drain can still fire; update resetProcessState() (which is called from handleSubprocessExit()) to call clearIdleEmulatorDrainTimer() just like dispose() does (before/around the code that clears emulatorWriteQueue/emulatorWriteScheduled) to cancel any pending drain timer and keep behavior symmetric with dispose().apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts (1)
151-164:disposeWebglAddon: 破棄経路でのxterm.refreshをスキップしても良いかも。
cleanup()経由で呼ばれた際は直後に xterm 自体が dispose されるため、xterm.refresh(0, ...)は不要なコストになります。disposedフラグを参照して破棄時の refresh を省略すると、複数 v1 ターミナルが同時破棄される workspace 切替シナリオで微小な負荷削減になります。また、
webglAddon.dispose()のtry { ... } catch {}は完全に静かに飲んでいるため、再現困難な GPU エラーを Sentry/terminalRendererDebug.warnに拾わせると今後のデバッグが楽になります。♻️ 提案差分
const disposeWebglAddon = () => { if (webglRafId !== null) { cancelAnimationFrame(webglRafId); webglRafId = null; } if (!webglAddon) return; try { webglAddon.dispose(); - } catch {} + } catch (error) { + terminalRendererDebug.warn("webgl-addon-dispose-failed", { + errorMessage: error instanceof Error ? error.message : String(error), + }); + } webglAddon = null; - if (opened) { + if (opened && !disposed) { xterm.refresh(0, Math.max(0, xterm.rows - 1)); } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts` around lines 151 - 164, The disposeWebglAddon function should skip calling xterm.refresh when the terminal is already being disposed and surface any unexpected errors from webglAddon.dispose to the renderer debug logger: add/consult a disposed flag (e.g. disposed) alongside opened and only run xterm.refresh if !disposed && opened, and change the empty catch around webglAddon.dispose() to report the caught error to terminalRendererDebug.warn (or Sentry) before continuing; keep cancelling webglRafId and nulling webglAddon as currently implemented.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/v1-terminal-cache.ts (1)
268-287: hidden 中バッファの上限がなく、高出力プロセスを含む pane を長時間隠したままにすると無制限に蓄積されます。
event.type === "data"を末尾の data イベントに連結する coalescing は順序を保ちつつ event 数を抑える点で目的に合致していますが、lastBufferedEvent.data自体のサイズ上限がないため、yes/ 連続ログ出力 /cat系で hidden のまま長時間放置すると renderer メモリが膨張し、再表示時に大きなxterm.writeが一括で走ることになります（PR 概要にある「再表示時の一時的スパイク」）。第二段の改善余地として、以下のいずれかを検討してください:
pendingUnmountedEventsの累計バイト数に上限（例: 数 MB）を設け、超えた場合は先頭の data event 部分を切り詰める（ターミナル UX 上、末尾を残すのが自然）。- 連結を
string += stringではなくstring[]+ 末尾結合に切り替え、再表示時のArray.prototype.joinで 1 回だけ確定させる（V8 の ConsString で実害が出にくいケースが多いものの、長時間蓄積では平坦化コストが累積し得るため）。第一段としては現状実装で機能するため、必須ではなく次段としての推奨です。
バイト数上限付きの先頭切り詰め実装を提案する PR コメント差分を出すこともできます。希望があればお知らせください。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/v1-terminal-cache.ts` around lines 268 - 287, The hidden-data buffering currently has no upper bound, causing unbounded memory growth in entry.pendingUnmountedEvents when appending to lastBufferedEvent.data; fix by adding a cumulative-byte cap and truncation: track a new numeric field (e.g., entry.pendingUnmountedBytes) and a constant MAX_PENDING_BYTES (few MB), increment it when pushing/appending data in the data branch inside the block that touches lastBufferedEvent and entry.pendingUnmountedEvents, and if the cap would be exceeded trim the oldest bytes from the start of the buffered data so the tail is preserved (or drop/clip the incoming chunk to fit). Also consider changing lastBufferedEvent.data from a monolithic string to an array of chunks (string[]) and only join on replay to avoid repeated string concatenation costs; ensure logTerminalWrite and terminalRendererDebug observations update based on the trimmed sizes.
🤖 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 1102-1136: resetProcessState currently doesn't clear the idle
emulator drain timer, so after a subprocess exit a pending idle drain can still
fire; update resetProcessState() (which is called from handleSubprocessExit())
to call clearIdleEmulatorDrainTimer() just like dispose() does (before/around
the code that clears emulatorWriteQueue/emulatorWriteScheduled) to cancel any
pending drain timer and keep behavior symmetric with dispose().
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts`:
- Around line 151-164: The disposeWebglAddon function should skip calling
xterm.refresh when the terminal is already being disposed and surface any
unexpected errors from webglAddon.dispose to the renderer debug logger:
add/consult a disposed flag (e.g. disposed) alongside opened and only run
xterm.refresh if !disposed && opened, and change the empty catch around
webglAddon.dispose() to report the caught error to terminalRendererDebug.warn
(or Sentry) before continuing; keep cancelling webglRafId and nulling webglAddon
as currently implemented.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/v1-terminal-cache.ts`:
- Around line 268-287: The hidden-data buffering currently has no upper bound,
causing unbounded memory growth in entry.pendingUnmountedEvents when appending
to lastBufferedEvent.data; fix by adding a cumulative-byte cap and truncation:
track a new numeric field (e.g., entry.pendingUnmountedBytes) and a constant
MAX_PENDING_BYTES (few MB), increment it when pushing/appending data in the data
branch inside the block that touches lastBufferedEvent and
entry.pendingUnmountedEvents, and if the cap would be exceeded trim the oldest
bytes from the start of the buffered data so the tail is preserved (or drop/clip
the incoming chunk to fit). Also consider changing lastBufferedEvent.data from a
monolithic string to an array of chunks (string[]) and only join on replay to
avoid repeated string concatenation costs; ensure logTerminalWrite and
terminalRendererDebug observations update based on the trimmed sizes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 69cfcd8a-55cd-46e8-80b5-a252094e3e7d
📒 Files selected for processing (5)
apps/desktop/src/main/terminal-host/session.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/hooks/useTerminalLifecycle.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/v1-terminal-cache.ts
![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: 36507bcf58
ℹ️ 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".
- resetProcessState: clear idle drain timer symmetrically with dispose() - disposeWebglAddon: skip xterm.refresh when disposed, surface GPU errors to debug logger - v1-terminal-cache: cap hidden buffer at 10MB, trim oldest data to preserve tail
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/v1-terminal-cache.ts (2)
466-470: dispose 時にpendingUnmountedEventsがクリアされていない
dispose(paneId)はcache.delete(paneId)を行うので参照が外れて GC 対象になりますが、明示的にentry.pendingUnmountedEvents = []; entry.pendingUnmountedBytes = 0;をクリアしておくと、HMR でhot.data.v1TerminalCacheを経由してエントリ参照が一時的に残るケースでメモリ保持時間を短縮できます（実害は軽微）。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/v1-terminal-cache.ts` around lines 466 - 470, In dispose(paneId) ensure you explicitly clear entry.pendingUnmountedEvents and entry.pendingUnmountedBytes before removing the entry from cache: locate the block where you disconnect observers and call entry.xterm.dispose() (the lines with entry.resizeObserver?.disconnect(), entry.subscription?.unsubscribe(), entry.cleanupCreation(), entry.xterm.dispose()) and add statements to set entry.pendingUnmountedEvents = [] and entry.pendingUnmountedBytes = 0 immediately prior to calling cache.delete(paneId) so any temporary HMR-held references (hot.data.v1TerminalCache) don't retain those buffers longer than necessary.
209-214:setFocusedのロジックはisAttached && isFocusedで適切に絞られている非 attach 中に focus イベントが届いても WebGL を起動しない設計になっており、
detachFromContainer時に明示的に false に倒すパスとも整合しています。ただし、focus が頻繁にトグルする UX（タブ切替や frequent autofocus）では WebGL の生成/破棄が繰り返される可能性があるため、本番動作で GPU リソースの再確保コストが体感できる場合は debounce の追加を検討する余地があります。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/v1-terminal-cache.ts` around lines 209 - 214, setFocused currently toggles GPU acceleration immediately via entry.setGpuAccelerationEnabled(entry.isAttached && isFocused), which can cause repeated WebGL create/destroy on rapid focus changes; modify setFocused to debounce the call to setGpuAccelerationEnabled (or add a short delay timer stored on the cache entry) so rapid toggles coalesce (e.g., 200–500ms) and ensure detachFromContainer still forces immediate disable by clearing the debounce and calling setGpuAccelerationEnabled(false) directly; reference functions/fields: setFocused, setGpuAccelerationEnabled, detachFromContainer, and the cache entry (entry.isAttached, entry.isFocused).apps/desktop/src/main/terminal-host/session.ts (1)
1069-1092:dispose()とresetProcessState()のタイマー解除が二重になっている点は問題なし（任意の整理）。
dispose()は L1073 でclearIdleEmulatorDrainTimer()を呼び、その後 L1081 のresetProcessState()内（L1126）でも同じ解除が走ります。clearIdleEmulatorDrainTimer()自体が冪等なので動作上の問題はありませんが、disposed = trueの直後に防御的に解除しておく意図がコメントで読み取れると保守者に親切です。♻️ 任意のコメント追加例
dispose(): Promise<void> { if (this.disposed) return Promise.resolve(); this.disposed = true; + // resetProcessState でも解除されるが、例外で到達しなかった場合に備え先に止める this.clearIdleEmulatorDrainTimer();🤖 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 1069 - 1092, dispose() currently calls clearIdleEmulatorDrainTimer() immediately after setting this.disposed and resetProcessState() calls the same timer-clear later; add a brief defensive comment in dispose() (near the this.disposed = true; and clearIdleEmulatorDrainTimer() calls) explaining that the early timer clear is intentional and defensive and that clearIdleEmulatorDrainTimer() is idempotent, and reference resetProcessState() to indicate the duplicate clear is expected for readability/maintainability.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts`:
- Around line 157-164: The catch block in disposeWebglAddon swallows the caught
error instead of logging it; update the catch to pass the caught error (the
error variable) into terminalRendererDebug.warn’s data payload (e.g., include
error.message and error.stack or the error object) so the actual GPU error
details are recorded; keep the existing fingerprint/captureMessage fields and
ensure terminalRendererDebug.warn is called with the error included for easier
debugging.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/v1-terminal-cache.ts`:
- Around line 282-289: The coalescing logic around entry.pendingUnmountedEvents
(check in v1-terminal-cache.ts where lastBufferedEvent?.type === "data" leads to
lastBufferedEvent.data += event.data and entry.pendingUnmountedBytes is
incremented, and where first.data = first.data.slice(excess) truncates the
front) can cut UTF-8 multibyte sequences or terminal escape/CSI sequences at
chunk boundaries and thus corrupt xterm's parser state on replay; add a clear
inline comment next to that coalescing/truncation block warning that: 1) this is
a rare 10MB-limit edge case, 2) slicing may split UTF-8/ESC/CSI sequences and
reverse/affect replay order relative to non-data events, and 3) recommend
possible mitigations (e.g., avoid coalescing across non-data events or ensure
slicing aligns to UTF-8/escape sequence boundaries) for future maintainers.
- Around line 292-308: The trimming loop can exit early when the head event is
non-"data" because it calls shift() then break; change the logic in the loop
that iterates while (entry.pendingUnmountedBytes > MAX_PENDING_UNMOUNTED_BYTES)
to skip non-data events instead of breaking: when first.type !== "data" simply
entry.pendingUnmountedEvents.shift() and continue so the loop re-checks the next
head, and only apply slicing/shifting to first.data when it contributes to
pendingUnmountedBytes; keep using entry.pendingUnmountedBytes,
MAX_PENDING_UNMOUNTED_BYTES and entry.pendingUnmountedEvents as the referenced
symbols.
---
Nitpick comments:
In `@apps/desktop/src/main/terminal-host/session.ts`:
- Around line 1069-1092: dispose() currently calls clearIdleEmulatorDrainTimer()
immediately after setting this.disposed and resetProcessState() calls the same
timer-clear later; add a brief defensive comment in dispose() (near the
this.disposed = true; and clearIdleEmulatorDrainTimer() calls) explaining that
the early timer clear is intentional and defensive and that
clearIdleEmulatorDrainTimer() is idempotent, and reference resetProcessState()
to indicate the duplicate clear is expected for readability/maintainability.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/v1-terminal-cache.ts`:
- Around line 466-470: In dispose(paneId) ensure you explicitly clear
entry.pendingUnmountedEvents and entry.pendingUnmountedBytes before removing the
entry from cache: locate the block where you disconnect observers and call
entry.xterm.dispose() (the lines with entry.resizeObserver?.disconnect(),
entry.subscription?.unsubscribe(), entry.cleanupCreation(),
entry.xterm.dispose()) and add statements to set entry.pendingUnmountedEvents =
[] and entry.pendingUnmountedBytes = 0 immediately prior to calling
cache.delete(paneId) so any temporary HMR-held references
(hot.data.v1TerminalCache) don't retain those buffers longer than necessary.
- Around line 209-214: setFocused currently toggles GPU acceleration immediately
via entry.setGpuAccelerationEnabled(entry.isAttached && isFocused), which can
cause repeated WebGL create/destroy on rapid focus changes; modify setFocused to
debounce the call to setGpuAccelerationEnabled (or add a short delay timer
stored on the cache entry) so rapid toggles coalesce (e.g., 200–500ms) and
ensure detachFromContainer still forces immediate disable by clearing the
debounce and calling setGpuAccelerationEnabled(false) directly; reference
functions/fields: setFocused, setGpuAccelerationEnabled, detachFromContainer,
and the cache entry (entry.isAttached, entry.isFocused).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9f98f4d4-ca72-4182-b51b-dbf5e31da64c
📒 Files selected for processing (3)
apps/desktop/src/main/terminal-host/session.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/v1-terminal-cache.ts
…ebGL teardown 事前レビューで指摘された HIGH 3 件と関連 nit を修正。 - v1-terminal-cache: 10MB 上限到達時の `slice(excess)` を廃止し、バッファ全捨て + RIS (`\x1bc`) 注入に切替。コアレス済み文字列を char index で切ると ANSI escape sequence や UTF-8 multi-byte 境界の中で分断され、xterm parser が壊れたまま replay されるため。10MB は long-lived hidden TUI の outlier ケースで、TUI は次の出力で全画面再描画する。 - v1-terminal-cache: subscription onError で `pendingUnmountedEvents` / `pendingUnmountedBytes` もクリア。これがないと remount 時に「エラー前」と「新 subscription」のイベントが混ざって xterm 状態が壊れる。 - helpers: scheduleWebglEnable と disposeWebglAddon の相互排他をコメントで明記。fast attach/detach toggle で二重 WebglAddon インスタンスが乗らないことの根拠 (rafId キャンセル + コールバック内ガード) を読者に伝える。 - helpers: webgl-addon-dispose-failed の warn payload に error.message を載せる。バインドした error 変数を捨てていたためテレメトリで原因が追えなかった。
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts (1)
214-222:onContextLossハンドラをdisposeWebglAddon経由に統一することを推奨。このハンドラは dispose と refresh を手書きで再実装しており、新規追加された
disposeWebglAddonと挙動が微妙にずれています。
- Line 215 の
webglAddon?.dispose()が try/catch されておらず、コミットメッセージで掲げた "surface GPU errors to debug logger" の方針から外れています（コンテキストロスト時の二次的な dispose 失敗が握りつぶされる、または例外として伝播する）。- Line 221 の
xterm.refresh(...)にはopened && !disposedのガードがありません。GPU プロセス側のクラッシュに伴ってcleanup()後にコンテキストロストが届くケースで、detach/dispose 済みの xterm に対して refresh が走り得ます。- ロジックが二箇所に分散することで、今後 dispose 周りに条件を追加した際にここが置き去りになりやすい構造になっています。
♻️ 提案: `disposeWebglAddon` に集約する
webglAddon.onContextLoss(() => { - webglAddon?.dispose(); - webglAddon = null; terminalRendererDebug.warn("webgl-context-lost", undefined, { captureMessage: true, fingerprint: ["terminal.renderer", "webgl-context-lost"], }); - xterm.refresh(0, Math.max(0, xterm.rows - 1)); + disposeWebglAddon(); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts` around lines 214 - 222, The onContextLoss handler should delegate to the existing disposeWebglAddon routine instead of duplicating dispose/refresh logic: replace the inline webglAddon?.dispose() + terminalRendererDebug.warn(...) + xterm.refresh(...) sequence with a call to disposeWebglAddon(webglAddon) (or call disposeWebglAddon() if it closes over webglAddon), ensuring disposeWebglAddon implements try/catch around addon.dispose(), emits the same terminalRendererDebug.warn("webgl-context-lost", ...) with captureMessage and fingerprint, and only calls xterm.refresh(0, Math.max(0, xterm.rows - 1)) when opened && !disposed; also ensure disposeWebglAddon clears webglAddon = null and retains any other cleanup behavior so all dispose semantics live in one place.
🤖 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/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts`:
- Around line 214-222: The onContextLoss handler should delegate to the existing
disposeWebglAddon routine instead of duplicating dispose/refresh logic: replace
the inline webglAddon?.dispose() + terminalRendererDebug.warn(...) +
xterm.refresh(...) sequence with a call to disposeWebglAddon(webglAddon) (or
call disposeWebglAddon() if it closes over webglAddon), ensuring
disposeWebglAddon implements try/catch around addon.dispose(), emits the same
terminalRendererDebug.warn("webgl-context-lost", ...) with captureMessage and
fingerprint, and only calls xterm.refresh(0, Math.max(0, xterm.rows - 1)) when
opened && !disposed; also ensure disposeWebglAddon clears webglAddon = null and
retains any other cleanup behavior so all dispose semantics live in one place.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 73d8e8e5-ee22-455f-854a-2e47fbe0ad34
📒 Files selected for processing (2)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/v1-terminal-cache.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/v1-terminal-cache.ts
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
概要
v1 ターミナルで Codex / Claude など更新頻度の高い TUI を複数開いたときの renderer/main process 負荷を軽くするための第一段です。
今回の主眼は、
Superset Helper (Renderer)の CPU が高止まりするケースに対して、見えていない terminal や非フォーカス pane が不要にxterm.write()/ WebGL renderer を回し続ける状態を減らすことです。変更内容
xterm.write()を継続せず、stream event を cache 側でバッファして remount 時に replay するように変更main 取り込み状況
origin/mainを取り込み済み78f0dd771 Merge remote-tracking branch 'origin/main' into codex/terminal-perf-1-3v1-terminal-cache.tsで debug 呼び出し形式のコンフリクトが発生したため、main 側の簡素化されたterminalRendererDebug.info(...)形式に合わせて解消entry.setGpuAccelerationEnabled(entry.isFocused)は残し、focus/attach 状態に応じた WebGL 制御は維持意図
renderer 側の負荷が主犯に見えるため、まず hidden terminal の処理を止める方針を優先しています。host 側の headless emulator は完全 lazy 化までは踏み込まず、attach / snapshot / shell 初期化の互換性を壊しにくい範囲で遅延バッチ化しています。
確認
git diff --check HEAD~1..HEADは問題なし注意点
Summary by CodeRabbit
リリースノート
新機能
バグ修正
パフォーマンス改善