From 76b13d4ef60dd3d5d8e166bacb71f27a4191631a Mon Sep 17 00:00:00 2001 From: "warp-agent-staging[bot]" <240773466+warp-agent-staging[bot]@users.noreply.github.com> Date: Tue, 18 Aug 2026 18:36:15 +0000 Subject: [PATCH 1/2] sum_tree: fix SumTree::push to merge into existing leaves Buffer::edit builds the full new content of a notebook/markdown buffer by calling SumTree::push once per style/block marker and newline while constructing the replacement content tree. SumTree::push wrapped every pushed item in its own single-item leaf plus a throwaway Internal node (via from_child_trees) before merging it in via push_tree. Once the tree grew past its first leaf, that extra Internal wrapper made push_tree_recursive treat the item as a same-height sibling subtree, so it appended a brand new one-item leaf instead of topping up the last leaf that still had room. Leaves are fixed-capacity ArrayVecs (2 * TREE_BASE slots), so a one-item leaf permanently allocates the same size as a full one. Every push() past the first few items therefore retained a mostly-empty leaf node for the lifetime of the tree, which is exactly what the Sentry heap profile for APP-5507 observed as ~9GB of retained inuse_space under SumTree::push/push_tree_recursive during FileNotebookView::open_local. Fix push() to hand push_tree a plain (unwrapped) single-item leaf, the same shape extend() already uses for its trailing partial leaf. This takes the height-delta descent path that merges into the last under-full leaf when there is room, instead of the flat same-height append path. Fixes APP-5507. --- crates/sum_tree/src/lib.rs | 20 +++++++++++------ crates/sum_tree/src/lib_tests.rs | 38 ++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 7 deletions(-) diff --git a/crates/sum_tree/src/lib.rs b/crates/sum_tree/src/lib.rs index b61fbef4b42..382df2dd008 100644 --- a/crates/sum_tree/src/lib.rs +++ b/crates/sum_tree/src/lib.rs @@ -153,13 +153,19 @@ impl SumTree { pub fn push(&mut self, item: T) { let summary = item.summary(); - self.push_tree(SumTree::from_child_trees(vec![SumTree(Arc::new( - Node::Leaf { - summary: summary.clone(), - items: ArrayVec::from_iter(Some(item)), - item_summaries: ArrayVec::from_iter(Some(summary)), - }, - ))])) + // Push a plain single-item leaf directly, rather than wrapping it in an extra + // `Internal` node via `from_child_trees`. That wrapping used to make `push_tree_recursive` + // treat the item as a same-height sibling subtree, which appends it as a brand-new + // one-item leaf instead of topping up the last leaf that still has room. Passing an + // unwrapped leaf here takes the same height-delta-based descent that `extend` already + // relies on for its final partial leaf, so single pushes merge into an existing + // under-full leaf whenever possible instead of permanently bloating the tree with + // singleton leaves (and skips one throwaway `Arc` allocation per push). + self.push_tree(SumTree(Arc::new(Node::Leaf { + summary: summary.clone(), + items: ArrayVec::from_iter(Some(item)), + item_summaries: ArrayVec::from_iter(Some(summary)), + }))) } pub fn push_tree(&mut self, other: Self) { diff --git a/crates/sum_tree/src/lib_tests.rs b/crates/sum_tree/src/lib_tests.rs index a51c3b3eb14..20a523c25e8 100644 --- a/crates/sum_tree/src/lib_tests.rs +++ b/crates/sum_tree/src/lib_tests.rs @@ -14,6 +14,44 @@ fn test_extend_and_push_tree() { assert_eq!(tree1.items(), (0..20).chain(50..100).collect::>()); } +#[test] +fn test_push_matches_extend_for_same_items() { + // TREE_BASE is 2 under the "test-util" feature (leaves hold up to 4 items), so 25 items + // forces multiple levels of the tree and several leaf splits. + let items: Vec = (0..25).collect(); + + let mut pushed = SumTree::new(); + for item in items.iter().copied() { + pushed.push(item); + } + + let mut extended = SumTree::new(); + extended.extend(items.clone()); + + assert_eq!(pushed.items(), items); + assert_eq!(pushed.items(), extended.items()); +} + +#[test] +fn test_repeated_push_packs_leaves_as_densely_as_extend() { + // Before the fix, `push` inserted every item as its own permanent one-item leaf once the + // tree grew past a single leaf, so tree height (and leaf/node count) grew linearly with the + // number of pushed items instead of logarithmically. Pushing one at a time should produce a + // tree exactly as densely packed - i.e. the same height - as building the same items via + // `extend`, which already batches items into full leaves. + let items: Vec = (0..25).collect(); + + let mut pushed = SumTree::new(); + for item in items.iter().copied() { + pushed.push(item); + } + + let mut extended = SumTree::new(); + extended.extend(items); + + assert_eq!(pushed.0.height(), extended.0.height()); +} + #[test] fn test_random() { for seed in 0..100 { From 8a534c1e8166b87f534af25aa76fdf3b8aaeaace Mon Sep 17 00:00:00 2001 From: "warp-agent-staging[bot]" <240773466+warp-agent-staging[bot]@users.noreply.github.com> Date: Tue, 18 Aug 2026 18:49:19 +0000 Subject: [PATCH 2/2] sum_tree: tighten push() comment to a present-tense property Remove the history-narrative comments flagged in review: state what the current code does (an unwrapped single-item leaf is always underflowing, so push_tree descends and tops up the last leaf) instead of describing what the old implementation used to do. --- crates/sum_tree/src/lib.rs | 10 ++-------- crates/sum_tree/src/lib_tests.rs | 5 ----- 2 files changed, 2 insertions(+), 13 deletions(-) diff --git a/crates/sum_tree/src/lib.rs b/crates/sum_tree/src/lib.rs index 382df2dd008..7279709a625 100644 --- a/crates/sum_tree/src/lib.rs +++ b/crates/sum_tree/src/lib.rs @@ -153,14 +153,8 @@ impl SumTree { pub fn push(&mut self, item: T) { let summary = item.summary(); - // Push a plain single-item leaf directly, rather than wrapping it in an extra - // `Internal` node via `from_child_trees`. That wrapping used to make `push_tree_recursive` - // treat the item as a same-height sibling subtree, which appends it as a brand-new - // one-item leaf instead of topping up the last leaf that still has room. Passing an - // unwrapped leaf here takes the same height-delta-based descent that `extend` already - // relies on for its final partial leaf, so single pushes merge into an existing - // under-full leaf whenever possible instead of permanently bloating the tree with - // singleton leaves (and skips one throwaway `Arc` allocation per push). + // An unwrapped single-item leaf is always underflowing, so `push_tree` descends into + // and tops up the last leaf that has room instead of appending a singleton sibling. self.push_tree(SumTree(Arc::new(Node::Leaf { summary: summary.clone(), items: ArrayVec::from_iter(Some(item)), diff --git a/crates/sum_tree/src/lib_tests.rs b/crates/sum_tree/src/lib_tests.rs index 20a523c25e8..ceb0a0b8485 100644 --- a/crates/sum_tree/src/lib_tests.rs +++ b/crates/sum_tree/src/lib_tests.rs @@ -34,11 +34,6 @@ fn test_push_matches_extend_for_same_items() { #[test] fn test_repeated_push_packs_leaves_as_densely_as_extend() { - // Before the fix, `push` inserted every item as its own permanent one-item leaf once the - // tree grew past a single leaf, so tree height (and leaf/node count) grew linearly with the - // number of pushed items instead of logarithmically. Pushing one at a time should produce a - // tree exactly as densely packed - i.e. the same height - as building the same items via - // `extend`, which already batches items into full leaves. let items: Vec = (0..25).collect(); let mut pushed = SumTree::new();