perf: wrap EditDelta.new_lines in Arc to avoid deep-cloning styled blocks#11631
Draft
warp-dev-github-integration[bot] wants to merge 1 commit into
Draft
perf: wrap EditDelta.new_lines in Arc to avoid deep-cloning styled blocks#11631warp-dev-github-integration[bot] wants to merge 1 commit into
warp-dev-github-integration[bot] wants to merge 1 commit into
Conversation
…ocks The EditDelta struct's new_lines field (Vec<StyledBufferBlock>) was being deep-cloned every time a content-change event was forwarded to the render pipeline. For large files loaded via Buffer::replace_all, this vector contains all styled blocks for the entire file content, causing ~1.8 GB of unnecessary allocations (24.7% of 7 GB total in-use memory from Sentry issue #7259255054). Changes: - Wrap EditDelta.new_lines in Arc<Vec<StyledBufferBlock>> so clones are reference-counted (O(1)) instead of deep-copying every styled block - Use Arc::try_unwrap in layout_delta() to avoid cloning when the Arc has a single owner (the common case in the layout pipeline) - Remove unnecessary delta.clone() in DelayRendering::flush_render where the delta was already owned from a consuming iterator - Update all construction sites to wrap in Arc::new() - Update test assertions to dereference Arc for comparisons 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
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.
Summary
Fixes excessive memory usage detected by Sentry issue #7259255054 (82 events, mac_stable_release / mac_preview_release).
Root Cause
The
EditDeltastruct'snew_linesfield (Vec<StyledBufferBlock>) was being deep-cloned every time a content-change event was forwarded to the render pipeline. For large files loaded viaBuffer::replace_all, this vector contains all styled blocks for the entire file content.Heap profile analysis showed ~7 GB total in-use memory with the following top allocators:
Buffer::replace_all→ SumTreeappend_str(buffer text replacement)Vec<StyledBufferBlock>::clone()inCodeEditorModel::handle_content_model_eventChanges
EditDelta.new_linesinArc<Vec<StyledBufferBlock>>— Cloning anEditDeltanow bumps a reference count (O(1)) instead of deep-copying every styled blockArc::try_unwrapinlayout_delta()— When the layout pipeline consumes the delta (typically the sole owner), it unwraps without cloning; falls back to clone only if there are multiple referencesdelta.clone()inDelayRendering::flush_render— The delta was already owned from a consuming iterator, making the clone redundantArc::new()Arcfor comparisonsExpected Impact
Vec<StyledBufferBlock>::clonepath)flush_rendercode path (additional unnecessary clone removed)Testing
buffer_tests.rsto work with theArcwrapperCHANGELOG-IMPROVEMENT:
CHANGELOG-BUG-FIX:
CHANGELOG-NEW-FEATURE:
CHANGELOG-IMAGE:
Conversation: https://staging.warp.dev/conversation/5505471b-acef-408a-92f0-799b1dc2186c
Run: https://oz.staging.warp.dev/runs/019e5971-7b78-7e2a-9e82-b127458e0218
This PR was generated with Oz.