fix: defer code review editor creation for collapsed files (APP-4518)#11381
Closed
warp-dev-github-integration[bot] wants to merge 1 commit into
Closed
fix: defer code review editor creation for collapsed files (APP-4518)#11381warp-dev-github-integration[bot] wants to merge 1 commit into
warp-dev-github-integration[bot] wants to merge 1 commit into
Conversation
Root cause: build_view_state_for_file_diffs eagerly created full editor view stacks (LocalCodeEditorView → CodeEditorView → CommentEditor → GlobalBufferModel) for every file in the diff set, including collapsed files (binary, autogenerated, large diffs). For large PRs this allocated multi-GB of styled buffer data that was never rendered. Fix: 1. Added deferred_content_at_head field to FileState to retain the base content for collapsed files without creating editor view stacks. 2. In build_view_state_for_file_diffs, skip editor creation for files where is_expanded == false; store content_at_head for later. 3. Added ensure_editor_for_file() which lazily creates the editor when the user first expands/selects a collapsed file. 4. Hooked ensure_editor_for_file() into ToggleFileExpanded and FileSelected action handlers. 5. Fixed FileInvalidationError::is_transient() to return false for permanent git failures (e.g. paths containing /null from submodule or tree entries) to avoid wasteful retries. Expected memory reduction: For a PR with N collapsed files, this avoids allocating ~50-100MB per file upfront. Based on the Sentry heap profile, this addresses the ~4-8 GiB from eager editor/buffer construction. Linked issues: - Sentry: https://warpdotdev.sentry.io/issues/7259255054/ - Linear: APP-4518 Co-Authored-By: Oz <oz-agent@warp.dev>
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
2 participants
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.
Description
Fix memory spike in code review diff rendering by deferring editor creation for collapsed files.
Root cause:
CodeReviewView::build_view_state_for_file_diffseagerly created full editor view stacks (LocalCodeEditorView→CodeEditorView→CommentEditor→GlobalBufferModel) for every file in the diff set, including collapsed files (binary, autogenerated, large diffs). The heap profile shows this path allocating 4–8 GiB on large diff sets.Fix:
deferred_content_at_headfield toFileStateto retain base content for collapsed files without creating editor stacks.build_view_state_for_file_diffs, skip editor creation whenis_expanded == false; storecontent_at_headfor later.ensure_editor_for_file(), which lazily creates the editor when the user first expands/selects a collapsed file.ensure_editor_for_file()intoToggleFileExpandedandFileSelectedhandlers.FileInvalidationError::is_transient()to returnfalsefor permanent git failures (paths containing/nullfrom submodule/tree entries) to avoid wasteful retries.Secondary finding: The
/nullpath failures in breadcrumbs (e.g.Could not access '.../step/04.1.z/null') are from git encountering tree objects where it expects blobs. These are correctly treated as binary (no editor created), but the retry queue was treating the failures as transient and retrying 3 times unnecessarily.This event is the same variant as APP-4518, not a new variant. PRs #11285 and #11281 target the same root cause but remain unmerged; this PR is an independent implementation.
Linked Issue
APP-4518
Sentry #7259255054
The linked issue is labeled
ready-to-specorready-to-implement.Where appropriate, screenshots or a short video of the implementation are included below (especially for user-visible or UI changes).
Testing
cargo check --package warp --testspasses ✅cargo fmt -- --checkpasses ✅cargo clippy -p warp --all-targets -- -D warningspasses ✅./script/runExisting unit tests in
code_review_view_tests.rsupdated to include the newdeferred_content_at_headfield.Agent Mode
CHANGELOG-BUG-FIX: Fixed memory spike (~4-8 GiB) when opening the code review panel with large diffs by deferring editor construction for collapsed files
Conversation: https://staging.warp.dev/conversation/bc5666e2-7409-47a9-843d-9413922dfc07
Run: https://oz.staging.warp.dev/runs/019e4375-3d99-762f-946f-3719f842f2d4
This PR was generated with Oz.