fix(core): throttle shell text output to prevent UI jank on high-volu… - #27010
fix(core): throttle shell text output to prevent UI jank on high-volu…#27010TirthNaik-99 wants to merge 1 commit into
Conversation
…me commands Apply OUTPUT_UPDATE_INTERVAL_MS (1s) throttling to text data events, matching the existing binary_progress throttle. This prevents thousands of React re-renders when commands emit high-volume output (builds, verbose test runs, npm install warnings). Key changes: - First text chunk renders immediately (lastUpdateTime starts at 0) - PTY (AnsiOutput) snapshots bypass throttle (already debounced at 68ms) - Trailing-edge flush ensures buffered output appears when command goes silent - Exit event flushes remaining output and cancels pending timers Fixes google-gemini#25459
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses performance degradation in the terminal UI when executing commands that produce high-volume text output. By applying a 1-second throttle to shell data events, the system avoids flooding the React render cycle, significantly improving UI responsiveness during long-running processes like builds or verbose test suites. The implementation includes robust handling for trailing flushes and process exits to ensure no output is lost, while specifically exempting PTY-based output to maintain expected terminal behavior. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
🛑 Action Required: Evaluation ApprovalSteering changes have been detected in this PR. To prevent regressions, a maintainer must approve the evaluation run before this PR can be merged. Maintainers:
Once approved, the evaluation results will be posted here automatically. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces output throttling for shell command execution in both the CLI UI and the core shell tool to improve performance during high-volume output. It implements a timer-based mechanism to ensure a final flush of data and adds comprehensive unit tests for these scenarios. The review feedback suggests ensuring that these timers are properly cleared in finally blocks to prevent potential memory leaks or unexpected state updates if an execution is aborted or encounters an error.
| if (trailingFlushTimer !== null) { | ||
| clearTimeout(trailingFlushTimer); | ||
| trailingFlushTimer = null; | ||
| } |
There was a problem hiding this comment.
The trailingFlushTimer should also be cleared in the finally block of the executeCommand function (around line 659). This ensures that no pending timers are left dangling if the command execution is aborted or encounters an error before completion, preventing potential state updates on an unmounted component or after the execution context has been cleaned up.
| if (trailingFlushTimer !== null) { | ||
| clearTimeout(trailingFlushTimer); | ||
| trailingFlushTimer = null; | ||
| } |
There was a problem hiding this comment.
The trailingFlushTimer should be cleared in the finally block of the execute method (around line 998). While it is cleared here during active updates, an early return or error in the execution flow could leave a timer pending, potentially calling updateOutput after the invocation has finished.
gemini-cli
Bot
added
priority/p2
|
we already have #26955 that is in the process of getting merged. |
3 participants
Shell tool
dataevents triggered a React re-render on every chunk whilebinary_progresswas already throttled to 1s intervals. Commands emitting thousands of lines pinned the UI until exit.Apply OUTPUT_UPDATE_INTERVAL_MS (1s) throttling to text data events, matching the existing binary_progress throttle. This prevents thousands of React re-renders when commands emit high-volume output (builds, verbose test runs, npm install warnings).
Key changes:
Fixes #25459
Summary
Apply
OUTPUT_UPDATE_INTERVAL_MS(1s) throttling to shell textdataevents in both the core tool (shell.ts) and the CLI UI hook (useExecutionLifecycle.ts), matching the existingbinary_progresscadence. High-volume commands (builds, verbose test runs) no longer pin the terminal UI with thousands of React re-renders.Details
lastUpdateTimestarts at0so the very first text chunk renders immediately (no initial 1s blank)AnsiOutputsnapshots bypass the throttle entirely — they are already debounced at 68ms inshellExecutionServicesetTimeoutflush ensures buffered output appears even when a command emits a burst then goes silentexitevent flushes remaining output and cancels any pending trailing timershellExecutionServiceafter the command exitsRelated Issues
Fixes #25459
How to Validate
npx vitest run packages/core/src/tools/shell.test.ts— all 82 tests pass (4 new regression tests)for i in $(seq 1 20000); do echo "line $i"; donePre-Merge Checklist