buffer_diff: Reduce number of entity clones in base_text_buffer (#49573)

Jakub Konka created 1 month ago

Release Notes:

- Improved split performance by reducing number of entity clones for
very large pre-loaded multibuffers.

Change summary

crates/buffer_diff/src/buffer_diff.rs         |  4 ++--
crates/editor/src/display_map/block_map.rs    |  2 +-
crates/editor/src/split.rs                    | 17 ++++++++++++-----
crates/multi_buffer/src/multi_buffer_tests.rs | 10 +++++-----
4 files changed, 20 insertions(+), 13 deletions(-)

Detailed changes

crates/buffer_diff/src/buffer_diff.rs

@@ -1978,8 +1978,8 @@ impl BufferDiff {
         cx.emit(BufferDiffEvent::DiffChanged(change));
     }
 
-    pub fn base_text_buffer(&self) -> Entity<language::Buffer> {
-        self.inner.base_text.clone()
+    pub fn base_text_buffer(&self) -> &Entity<language::Buffer> {
+        &self.inner.base_text
     }
 }
 

crates/editor/src/display_map/block_map.rs

@@ -4591,7 +4591,7 @@ mod tests {
         let diff = cx.new(|cx| {
             BufferDiff::new_with_base_text(base_text, &rhs_buffer.read(cx).text_snapshot(), cx)
         });
-        let lhs_buffer = diff.read_with(cx, |diff, _| diff.base_text_buffer());
+        let lhs_buffer = diff.read_with(cx, |diff, _| diff.base_text_buffer().clone());
 
         let lhs_multibuffer = cx.new(|cx| {
             let mut mb = MultiBuffer::new(Capability::ReadWrite);

crates/editor/src/split.rs

@@ -2027,9 +2027,16 @@ impl LhsEditor {
         let main_buffer = rhs_multibuffer_snapshot
             .buffer_for_excerpt(excerpt_id)
             .unwrap();
-        let base_text_buffer = diff.read(lhs_cx).base_text_buffer();
-        let diff_snapshot = diff.read(lhs_cx).snapshot(lhs_cx);
-        let base_text_buffer_snapshot = base_text_buffer.read(lhs_cx).snapshot();
+        let diff_snapshot;
+        let base_text_buffer_snapshot;
+        let remote_id;
+        {
+            let diff = diff.read(lhs_cx);
+            let base_text_buffer = diff.base_text_buffer().read(lhs_cx);
+            diff_snapshot = diff.snapshot(lhs_cx);
+            base_text_buffer_snapshot = base_text_buffer.snapshot();
+            remote_id = base_text_buffer.remote_id();
+        }
         let excerpt_ranges = rhs_multibuffer
             .excerpts_for_buffer(main_buffer.remote_id(), lhs_cx)
             .into_iter()
@@ -2069,7 +2076,7 @@ impl LhsEditor {
             .collect::<Vec<_>>();
         let lhs_result = lhs_multibuffer.set_merged_excerpt_ranges_for_path(
             path_key,
-            base_text_buffer.clone(),
+            diff.read(lhs_cx).base_text_buffer().clone(),
             excerpt_ranges,
             &base_text_buffer_snapshot,
             new,
@@ -2078,7 +2085,7 @@ impl LhsEditor {
         );
         if !lhs_result.excerpt_ids.is_empty()
             && lhs_multibuffer
-                .diff_for(base_text_buffer.read(lhs_cx).remote_id())
+                .diff_for(remote_id)
                 .is_none_or(|old_diff| old_diff.entity_id() != diff.entity_id())
         {
             lhs_multibuffer.add_inverted_diff(diff, lhs_cx);

crates/multi_buffer/src/multi_buffer_tests.rs

@@ -521,7 +521,7 @@ async fn test_inverted_diff_hunks_in_range(cx: &mut TestAppContext) {
     let buffer = cx.new(|cx| Buffer::local(text, cx));
     let diff = cx
         .new(|cx| BufferDiff::new_with_base_text(base_text, &buffer.read(cx).text_snapshot(), cx));
-    let base_text_buffer = diff.read_with(cx, |diff, _| diff.base_text_buffer());
+    let base_text_buffer = diff.read_with(cx, |diff, _| diff.base_text_buffer().clone());
     let multibuffer = cx.new(|cx| MultiBuffer::singleton(base_text_buffer.clone(), cx));
     let (mut snapshot, mut subscription) = multibuffer.update(cx, |multibuffer, cx| {
         (multibuffer.snapshot(cx), multibuffer.subscribe())
@@ -3066,7 +3066,7 @@ async fn test_random_multibuffer(cx: &mut TestAppContext, mut rng: StdRng) {
                         });
 
                         let base_text_buffer =
-                            diff.read_with(cx, |diff, _| diff.base_text_buffer());
+                            diff.read_with(cx, |diff, _| diff.base_text_buffer().clone());
 
                         // Track for recalculation when main buffer is edited
                         inverted_diff_main_buffers.insert(main_buffer_id, diff.clone());
@@ -3100,7 +3100,7 @@ async fn test_random_multibuffer(cx: &mut TestAppContext, mut rng: StdRng) {
 
                     if let Some(diff) = inverted_diff_main_buffers.get(&buffer_id) {
                         let base_text_buffer =
-                            diff.read_with(cx, |diff, _| diff.base_text_buffer());
+                            diff.read_with(cx, |diff, _| diff.base_text_buffer().clone());
                         (base_text_buffer, diff.clone(), Some(buffer_handle))
                     } else {
                         // Get existing diff or create new one for regular buffer
@@ -3933,7 +3933,7 @@ async fn test_singleton_with_inverted_diff(cx: &mut TestAppContext) {
         .new(|cx| BufferDiff::new_with_base_text(base_text, &buffer.read(cx).text_snapshot(), cx));
     cx.run_until_parked();
 
-    let base_text_buffer = diff.read_with(cx, |diff, _| diff.base_text_buffer());
+    let base_text_buffer = diff.read_with(cx, |diff, _| diff.base_text_buffer().clone());
 
     let multibuffer = cx.new(|cx| {
         let mut multibuffer = MultiBuffer::singleton(base_text_buffer.clone(), cx);
@@ -4084,7 +4084,7 @@ async fn test_inverted_diff_base_text_change(cx: &mut TestAppContext) {
         .new(|cx| BufferDiff::new_with_base_text(base_text, &buffer.read(cx).text_snapshot(), cx));
     cx.run_until_parked();
 
-    let base_text_buffer = diff.read_with(cx, |diff, _| diff.base_text_buffer());
+    let base_text_buffer = diff.read_with(cx, |diff, _| diff.base_text_buffer().clone());
 
     let multibuffer = cx.new(|cx| {
         let mut multibuffer = MultiBuffer::singleton(base_text_buffer.clone(), cx);