From cede296b04d2717ca74fe212d89acef95c56a61e Mon Sep 17 00:00:00 2001 From: Julia Date: Thu, 18 May 2023 18:25:58 -0400 Subject: [PATCH] Project level git diff recalc handling 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 --- 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(-) diff --git a/crates/editor/src/multi_buffer.rs b/crates/editor/src/multi_buffer.rs index 6b1ad6c5b2d6eb7c2e7bd9be9376b5fad04b8dd7..e749f5dcfb077a0d93d81d6329aa975b24752686 100644 --- a/crates/editor/src/multi_buffer.rs +++ b/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) { - 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) { + // 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( diff --git a/crates/language/src/buffer.rs b/crates/language/src/buffer.rs index 93955a247482d09eb1155f530f644fcaee62162b..f6eb2a5d2d2813890af69c06b6a45cc2e2d2e9c3 100644 --- a/crates/language/src/buffer.rs +++ b/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) -> Option> { - let diff_base = &self.diff_base?; + pub fn git_diff_recalc_2(&mut self, cx: &mut ModelContext) -> Option> { + 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) { + fn git_diff_recalc(&mut self, cx: &mut ModelContext) { if self.git_diff_status.update_in_progress { self.git_diff_status.update_requested = true; return; diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index f93d7d4b5051480b92a3bedf30b2633d36ac6741..3f520c7e9924f73239881852abb199904bed22b8 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -1610,6 +1610,7 @@ impl Project { buffer: &ModelHandle, cx: &mut ModelContext, ) -> 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, ) -> 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, + cx: &mut ModelContext, + ) { + 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) { 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.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) }) } diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index d2c035f91675a779b84a4facfc6331091b481910..3a7044de0c705be6084401c2f90e9ef2b007e1d7 100644 --- a/crates/project/src/worktree.rs +++ b/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))))) }) }