Project level git diff recalc handling

Julia and Max Brunsfeld created 2 years ago

This avoids an issue where in a many-buffer multi-buffer, each modified
buffer could complete its recalc independently, causing a cascade of
repeated notifies

Now all recalcs started at the same time must complete before
 A: Starting another recalc pass
 B: The master notify occurring

Each buffer can still show its new diff if something else triggers it
to notify earlier, this is desirable and does not have the same negative
effects as the notify cascade as those re-layouts would need to happen
anyway

Co-Authored-By: Max Brunsfeld <[e-mail]>

Change summary

crates/editor/src/multi_buffer.rs | 18 +++++-----
crates/language/src/buffer.rs     | 22 +++++++++----
crates/project/src/project.rs     | 55 +++++++++++++++++++++++---------
crates/project/src/worktree.rs    |  6 ---
4 files changed, 64 insertions(+), 37 deletions(-)

Detailed changes

crates/editor/src/multi_buffer.rs

@@ -343,15 +343,15 @@ impl MultiBuffer {
         self.read(cx).symbols_containing(offset, theme)
     }
 
-    pub fn git_diff_recalc(&mut self, cx: &mut ModelContext<Self>) {
-        let buffers = self.buffers.borrow();
-        for buffer_state in buffers.values() {
-            if buffer_state.buffer.read(cx).needs_git_diff_recalc() {
-                buffer_state
-                    .buffer
-                    .update(cx, |buffer, cx| buffer.git_diff_recalc(cx))
-            }
-        }
+    pub fn git_diff_recalc(&mut self, _: &mut ModelContext<Self>) {
+        // let buffers = self.buffers.borrow();
+        // for buffer_state in buffers.values() {
+        //     if buffer_state.buffer.read(cx).needs_git_diff_recalc() {
+        //         buffer_state
+        //             .buffer
+        //             .update(cx, |buffer, cx| buffer.git_diff_recalc(cx))
+        //     }
+        // }
     }
 
     pub fn edit<I, S, T>(

crates/language/src/buffer.rs

@@ -682,21 +682,29 @@ impl Buffer {
         self.git_diff_status.diff.needs_update(self)
     }
 
-    fn git_diff_recalc_2(&mut self, cx: &mut ModelContext<Self>) -> Option<Task<()>> {
-        let diff_base = &self.diff_base?;
+    pub fn git_diff_recalc_2(&mut self, cx: &mut ModelContext<Self>) -> Option<Task<()>> {
+        let diff_base = self.diff_base.clone()?; // TODO: Make this an Arc
         let snapshot = self.snapshot();
-        let handle = cx.weak_handle();
 
         let mut diff = self.git_diff_status.diff.clone();
-        Some(cx.background().spawn(async move {
+        let diff = cx.background().spawn(async move {
             diff.update(&diff_base, &snapshot).await;
-            if let Some(this) = handle.upgrade(cx) {
-                // this.update(cx)
+            diff
+        });
+
+        let handle = cx.weak_handle();
+        Some(cx.spawn_weak(|_, mut cx| async move {
+            let buffer_diff = diff.await;
+            if let Some(this) = handle.upgrade(&mut cx) {
+                this.update(&mut cx, |this, _| {
+                    this.git_diff_status.diff = buffer_diff;
+                    this.git_diff_update_count += 1;
+                })
             }
         }))
     }
 
-    pub fn git_diff_recalc(&mut self, cx: &mut ModelContext<Self>) {
+    fn git_diff_recalc(&mut self, cx: &mut ModelContext<Self>) {
         if self.git_diff_status.update_in_progress {
             self.git_diff_status.update_requested = true;
             return;

crates/project/src/project.rs

@@ -1610,6 +1610,7 @@ impl Project {
         buffer: &ModelHandle<Buffer>,
         cx: &mut ModelContext<Self>,
     ) -> Result<()> {
+        self.request_buffer_diff_recalculation(buffer, cx);
         buffer.update(cx, |buffer, _| {
             buffer.set_language_registry(self.languages.clone())
         });
@@ -1928,17 +1929,7 @@ impl Project {
         cx: &mut ModelContext<Self>,
     ) -> Option<()> {
         if matches!(event, BufferEvent::Edited { .. } | BufferEvent::Reloaded) {
-            self.buffers_needing_diff.insert(buffer.downgrade());
-            if self.buffers_needing_diff.len() == 1 {
-                let this = cx.weak_handle();
-                cx.defer(move |cx| {
-                    if let Some(this) = this.upgrade(cx) {
-                        this.update(cx, |this, cx| {
-                            this.recalculate_buffer_diffs(cx);
-                        });
-                    }
-                });
-            }
+            self.request_buffer_diff_recalculation(&buffer, cx);
         }
 
         match event {
@@ -2080,11 +2071,33 @@ impl Project {
         None
     }
 
+    fn request_buffer_diff_recalculation(
+        &mut self,
+        buffer: &ModelHandle<Buffer>,
+        cx: &mut ModelContext<Self>,
+    ) {
+        self.buffers_needing_diff.insert(buffer.downgrade());
+        if self.buffers_needing_diff.len() == 1 {
+            let this = cx.weak_handle();
+            cx.defer(move |cx| {
+                if let Some(this) = this.upgrade(cx) {
+                    this.update(cx, |this, cx| {
+                        this.recalculate_buffer_diffs(cx);
+                    });
+                }
+            });
+        }
+    }
+
     fn recalculate_buffer_diffs(&mut self, cx: &mut ModelContext<Self>) {
         cx.spawn(|this, mut cx| async move {
-            let tasks: Vec<_> = this.update(&mut cx, |this, cx| {
-                this.buffers_needing_diff
-                    .drain()
+            let buffers: Vec<_> = this.update(&mut cx, |this, _| {
+                this.buffers_needing_diff.drain().collect()
+            });
+
+            let tasks: Vec<_> = this.update(&mut cx, |_, cx| {
+                buffers
+                    .iter()
                     .filter_map(|buffer| {
                         let buffer = buffer.upgrade(cx)?;
                         buffer.update(cx, |buffer, cx| buffer.git_diff_recalc_2(cx))
@@ -2092,11 +2105,18 @@ impl Project {
                     .collect()
             });
 
-            let updates = futures::future::join_all(tasks).await;
+            futures::future::join_all(tasks).await;
 
             this.update(&mut cx, |this, cx| {
                 if !this.buffers_needing_diff.is_empty() {
                     this.recalculate_buffer_diffs(cx);
+                } else {
+                    // TODO: Would a `ModelContext<Project>.notify()` suffice here?
+                    for buffer in buffers {
+                        if let Some(buffer) = buffer.upgrade(cx) {
+                            buffer.update(cx, |_, cx| cx.notify());
+                        }
+                    }
                 }
             });
         })
@@ -6229,11 +6249,13 @@ impl Project {
                 let Some(this) = this.upgrade(&cx) else {
                     return Err(anyhow!("project dropped"));
                 };
+
                 let buffer = this.read_with(&cx, |this, cx| {
                     this.opened_buffers
                         .get(&id)
                         .and_then(|buffer| buffer.upgrade(cx))
                 });
+
                 if let Some(buffer) = buffer {
                     break buffer;
                 } else if this.read_with(&cx, |this, _| this.is_read_only()) {
@@ -6244,12 +6266,13 @@ impl Project {
                     this.incomplete_remote_buffers.entry(id).or_default();
                 });
                 drop(this);
+
                 opened_buffer_rx
                     .next()
                     .await
                     .ok_or_else(|| anyhow!("project dropped while waiting for buffer"))?;
             };
-            buffer.update(&mut cx, |buffer, cx| buffer.git_diff_recalc(cx));
+
             Ok(buffer)
         })
     }

crates/project/src/worktree.rs

@@ -719,11 +719,7 @@ impl LocalWorktree {
                 .background()
                 .spawn(async move { text::Buffer::new(0, id, contents) })
                 .await;
-            Ok(cx.add_model(|cx| {
-                let mut buffer = Buffer::build(text_buffer, diff_base, Some(Arc::new(file)));
-                buffer.git_diff_recalc(cx);
-                buffer
-            }))
+            Ok(cx.add_model(|_| Buffer::build(text_buffer, diff_base, Some(Arc::new(file)))))
         })
     }