text: Preserve structural sharing when rebuilding the fragment tree - #58681
Merged
Conversation
Lets the find/replace benchmark exercise the pessimistic case for a from-scratch rebuild: a single tiny edit applied to a huge buffer, where the cost is dominated by how the fragment tree is rebuilt rather than by the edit itself. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
apply_remote_edit and apply_local_edit rebuild the fragment SumTree through FragmentBuilder. Since #51941 the builder collapsed the entire tree into a flat Vec<Fragment> (cloning every fragment) and rebuilt a fresh tree from scratch on every call. The new tree shared no nodes with the old one, so each edit cost O(N) in the total number of fragments: clone all fragments, allocate an entirely new tree, and then free the whole previous tree on assignment (the latter showing up as a large amount of time spent dropping SumTrees). Make FragmentBuilder chunk-based instead: appended slices are kept as intact SumTree subtrees so they keep sharing nodes with the previous tree, and only individually pushed fragments are batched into Vecs. to_sum_tree appends the shared subtrees (touching just the right spine) and builds the loose runs in one pass, parallelizing large ones. Small edits on large buffers are back to O(edited + log N) with a cheap drop, and the bulk replace_all path is unaffected. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
cla-bot
Bot
added
the
cla-signed
zed-community-bot
Bot
added
the
staff
Member
|
@zed-zippy approve |
TomPlanche
pushed a commit
to TomPlanche/zed
that referenced
this pull request
Jun 8, 2026
…ed-industries#58681) ## Context `apply_remote_edit` and `apply_local_edit` rebuild the fragment `SumTree` through `FragmentBuilder`. Since zed-industries#51941, the builder collapsed the entire tree into a flat `Vec<Fragment>` (cloning every fragment) and rebuilt a fresh tree from scratch on every call. The new tree shared no nodes with the previous one, so each edit cost O(N) in the *total* number of fragments: clone all fragments, allocate an entirely new tree, and then free the whole previous tree on assignment — the last part showing up as a large amount of time spent dropping `SumTree`s. That batching was a deliberate win for the bulk `replace_all` path (zed-industries#51941, one `edit()` carrying millions of ranges), but it penalizes every other caller — most notably `apply_remote_edit`, which runs once per op in a loop in `apply_ops` and so rebuilt the whole tree per remote operation. ## This change Make `FragmentBuilder` chunk-based. Appended slices are kept as intact `SumTree` subtrees, so they keep sharing nodes with the previous tree; only individually pushed fragments are batched into `Vec`s. `to_sum_tree` appends the shared subtrees (touching just the right spine) and builds the loose runs in one pass, parallelizing the large ones. Net effect: - Small edits on large/heavily-fragmented buffers (the `apply_remote_edit` collaborative path) go back to O(edited + log N) with a cheap drop, instead of O(total fragments). - The bulk `replace_all` path keeps its batched build and is not regressed. ## Benchmarks Using the file from zed-industries#38927 (619 MB CSV, 10,325,246 matches of `"` → `""`), `release-fast`: | Case | flatten (zed-industries#51941) | this PR | |---|---|---| | `replace_all` (all matches) | 50.70 s | 45.16 s | | replace 1 match (`--single`) | 16.0 ms | 16.0 ms | `replace_all` is ~11% faster and not regressed. The single-match-on-a-freshly-loaded-file case is unchanged, because a fresh `Buffer::local` is barely fragmented, so even the from-scratch rebuild is cheap there; the structural-sharing win is on heavily-fragmented buffers receiving small edits. The `--single` flag added to `editor_benchmarks` makes the latter case measurable. Release Notes: - Improved the performance of applying edits to large buffers --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This was referenced Jun 18, 2026
Closed
This was referenced Jul 1, 2026
This was referenced Jul 10, 2026
jonx
pushed a commit
to jonx/zed-aros
that referenced
this pull request
Jul 17, 2026
…ed-industries#58681) ## Context `apply_remote_edit` and `apply_local_edit` rebuild the fragment `SumTree` through `FragmentBuilder`. Since zed-industries#51941, the builder collapsed the entire tree into a flat `Vec<Fragment>` (cloning every fragment) and rebuilt a fresh tree from scratch on every call. The new tree shared no nodes with the previous one, so each edit cost O(N) in the *total* number of fragments: clone all fragments, allocate an entirely new tree, and then free the whole previous tree on assignment — the last part showing up as a large amount of time spent dropping `SumTree`s. That batching was a deliberate win for the bulk `replace_all` path (zed-industries#51941, one `edit()` carrying millions of ranges), but it penalizes every other caller — most notably `apply_remote_edit`, which runs once per op in a loop in `apply_ops` and so rebuilt the whole tree per remote operation. ## This change Make `FragmentBuilder` chunk-based. Appended slices are kept as intact `SumTree` subtrees, so they keep sharing nodes with the previous tree; only individually pushed fragments are batched into `Vec`s. `to_sum_tree` appends the shared subtrees (touching just the right spine) and builds the loose runs in one pass, parallelizing the large ones. Net effect: - Small edits on large/heavily-fragmented buffers (the `apply_remote_edit` collaborative path) go back to O(edited + log N) with a cheap drop, instead of O(total fragments). - The bulk `replace_all` path keeps its batched build and is not regressed. ## Benchmarks Using the file from zed-industries#38927 (619 MB CSV, 10,325,246 matches of `"` → `""`), `release-fast`: | Case | flatten (zed-industries#51941) | this PR | |---|---|---| | `replace_all` (all matches) | 50.70 s | 45.16 s | | replace 1 match (`--single`) | 16.0 ms | 16.0 ms | `replace_all` is ~11% faster and not regressed. The single-match-on-a-freshly-loaded-file case is unchanged, because a fresh `Buffer::local` is barely fragmented, so even the from-scratch rebuild is cheap there; the structural-sharing win is on heavily-fragmented buffers receiving small edits. The `--single` flag added to `editor_benchmarks` makes the latter case measurable. Release Notes: - Improved the performance of applying edits to large buffers --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
jolutz
pushed a commit
to jolutz/zed
that referenced
this pull request
Aug 8, 2026
…ed-industries#58681) ## Context `apply_remote_edit` and `apply_local_edit` rebuild the fragment `SumTree` through `FragmentBuilder`. Since zed-industries#51941, the builder collapsed the entire tree into a flat `Vec<Fragment>` (cloning every fragment) and rebuilt a fresh tree from scratch on every call. The new tree shared no nodes with the previous one, so each edit cost O(N) in the *total* number of fragments: clone all fragments, allocate an entirely new tree, and then free the whole previous tree on assignment — the last part showing up as a large amount of time spent dropping `SumTree`s. That batching was a deliberate win for the bulk `replace_all` path (zed-industries#51941, one `edit()` carrying millions of ranges), but it penalizes every other caller — most notably `apply_remote_edit`, which runs once per op in a loop in `apply_ops` and so rebuilt the whole tree per remote operation. ## This change Make `FragmentBuilder` chunk-based. Appended slices are kept as intact `SumTree` subtrees, so they keep sharing nodes with the previous tree; only individually pushed fragments are batched into `Vec`s. `to_sum_tree` appends the shared subtrees (touching just the right spine) and builds the loose runs in one pass, parallelizing the large ones. Net effect: - Small edits on large/heavily-fragmented buffers (the `apply_remote_edit` collaborative path) go back to O(edited + log N) with a cheap drop, instead of O(total fragments). - The bulk `replace_all` path keeps its batched build and is not regressed. ## Benchmarks Using the file from zed-industries#38927 (619 MB CSV, 10,325,246 matches of `"` → `""`), `release-fast`: | Case | flatten (zed-industries#51941) | this PR | |---|---|---| | `replace_all` (all matches) | 50.70 s | 45.16 s | | replace 1 match (`--single`) | 16.0 ms | 16.0 ms | `replace_all` is ~11% faster and not regressed. The single-match-on-a-freshly-loaded-file case is unchanged, because a fresh `Buffer::local` is barely fragmented, so even the from-scratch rebuild is cheap there; the structural-sharing win is on heavily-fragmented buffers receiving small edits. The `--single` flag added to `editor_benchmarks` makes the latter case measurable. Release Notes: - Improved the performance of applying edits to large buffers --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
Context
apply_remote_editandapply_local_editrebuild the fragmentSumTreethroughFragmentBuilder. Since #51941, the builder collapsed the entire tree into a flatVec<Fragment>(cloning every fragment) and rebuilt a fresh tree from scratch on every call. The new tree shared no nodes with the previous one, so each edit cost O(N) in the total number of fragments: clone all fragments, allocate an entirely new tree, and then free the whole previous tree on assignment — the last part showing up as a large amount of time spent droppingSumTrees.That batching was a deliberate win for the bulk
replace_allpath (#51941, oneedit()carrying millions of ranges), but it penalizes every other caller — most notablyapply_remote_edit, which runs once per op in a loop inapply_opsand so rebuilt the whole tree per remote operation.This change
Make
FragmentBuilderchunk-based. Appended slices are kept as intactSumTreesubtrees, so they keep sharing nodes with the previous tree; only individually pushed fragments are batched intoVecs.to_sum_treeappends the shared subtrees (touching just the right spine) and builds the loose runs in one pass, parallelizing the large ones.Net effect:
apply_remote_editcollaborative path) go back to O(edited + log N) with a cheap drop, instead of O(total fragments).replace_allpath keeps its batched build and is not regressed.Benchmarks
Using the file from #38927 (619 MB CSV, 10,325,246 matches of
"→""),release-fast:replace_all(all matches)--single)replace_allis ~11% faster and not regressed. The single-match-on-a-freshly-loaded-file case is unchanged, because a freshBuffer::localis barely fragmented, so even the from-scratch rebuild is cheap there; the structural-sharing win is on heavily-fragmented buffers receiving small edits.The
--singleflag added toeditor_benchmarksmakes the latter case measurable.Release Notes: