Remove excessive hint update queries

* Filter out queries for outdated buffers just before hint tasks spawn:
  multicared edits might empit standalone events simultaneously
* Only spawn inlay update tasks for visible buffers with corresponding
  language
* Do not spawn tasks for local projects' buffers without LSP servers
This commit is contained in:
Kirill Bulatov 2023-06-30 08:19:57 +03:00
parent f83514cde4
commit ae54e1d224
3 changed files with 405 additions and 107 deletions

View File

@ -26,7 +26,7 @@ use aho_corasick::AhoCorasick;
use anyhow::{anyhow, Result}; use anyhow::{anyhow, Result};
use blink_manager::BlinkManager; use blink_manager::BlinkManager;
use client::{ClickhouseEvent, TelemetrySettings}; use client::{ClickhouseEvent, TelemetrySettings};
use clock::ReplicaId; use clock::{Global, ReplicaId};
use collections::{BTreeMap, Bound, HashMap, HashSet, VecDeque}; use collections::{BTreeMap, Bound, HashMap, HashSet, VecDeque};
use copilot::Copilot; use copilot::Copilot;
pub use display_map::DisplayPoint; pub use display_map::DisplayPoint;
@ -1195,11 +1195,11 @@ enum GotoDefinitionKind {
Type, Type,
} }
#[derive(Debug, Copy, Clone)] #[derive(Debug, Clone)]
enum InlayRefreshReason { enum InlayRefreshReason {
SettingsChange(InlayHintSettings), SettingsChange(InlayHintSettings),
NewLinesShown, NewLinesShown,
ExcerptEdited, BufferEdited(HashSet<Arc<Language>>),
RefreshRequested, RefreshRequested,
} }
@ -2617,7 +2617,7 @@ impl Editor {
return; return;
} }
let invalidate_cache = match reason { let (invalidate_cache, required_languages) = match reason {
InlayRefreshReason::SettingsChange(new_settings) => { InlayRefreshReason::SettingsChange(new_settings) => {
match self.inlay_hint_cache.update_settings( match self.inlay_hint_cache.update_settings(
&self.buffer, &self.buffer,
@ -2633,16 +2633,18 @@ impl Editor {
return; return;
} }
ControlFlow::Break(None) => return, ControlFlow::Break(None) => return,
ControlFlow::Continue(()) => InvalidationStrategy::RefreshRequested, ControlFlow::Continue(()) => (InvalidationStrategy::RefreshRequested, None),
} }
} }
InlayRefreshReason::NewLinesShown => InvalidationStrategy::None, InlayRefreshReason::NewLinesShown => (InvalidationStrategy::None, None),
InlayRefreshReason::ExcerptEdited => InvalidationStrategy::ExcerptEdited, InlayRefreshReason::BufferEdited(buffer_languages) => {
InlayRefreshReason::RefreshRequested => InvalidationStrategy::RefreshRequested, (InvalidationStrategy::BufferEdited, Some(buffer_languages))
}
InlayRefreshReason::RefreshRequested => (InvalidationStrategy::RefreshRequested, None),
}; };
self.inlay_hint_cache.refresh_inlay_hints( self.inlay_hint_cache.refresh_inlay_hints(
self.excerpt_visible_offsets(cx), self.excerpt_visible_offsets(required_languages.as_ref(), cx),
invalidate_cache, invalidate_cache,
cx, cx,
) )
@ -2661,8 +2663,9 @@ impl Editor {
fn excerpt_visible_offsets( fn excerpt_visible_offsets(
&self, &self,
restrict_to_languages: Option<&HashSet<Arc<Language>>>,
cx: &mut ViewContext<'_, '_, Editor>, cx: &mut ViewContext<'_, '_, Editor>,
) -> HashMap<ExcerptId, (ModelHandle<Buffer>, Range<usize>)> { ) -> HashMap<ExcerptId, (ModelHandle<Buffer>, Global, Range<usize>)> {
let multi_buffer = self.buffer().read(cx); let multi_buffer = self.buffer().read(cx);
let multi_buffer_snapshot = multi_buffer.snapshot(cx); let multi_buffer_snapshot = multi_buffer.snapshot(cx);
let multi_buffer_visible_start = self let multi_buffer_visible_start = self
@ -2680,8 +2683,22 @@ impl Editor {
.range_to_buffer_ranges(multi_buffer_visible_range, cx) .range_to_buffer_ranges(multi_buffer_visible_range, cx)
.into_iter() .into_iter()
.filter(|(_, excerpt_visible_range, _)| !excerpt_visible_range.is_empty()) .filter(|(_, excerpt_visible_range, _)| !excerpt_visible_range.is_empty())
.map(|(buffer, excerpt_visible_range, excerpt_id)| { .filter_map(|(buffer_handle, excerpt_visible_range, excerpt_id)| {
(excerpt_id, (buffer, excerpt_visible_range)) let buffer = buffer_handle.read(cx);
let language = buffer.language()?;
if let Some(restrict_to_languages) = restrict_to_languages {
if !restrict_to_languages.contains(language) {
return None;
}
}
Some((
excerpt_id,
(
buffer_handle,
buffer.version().clone(),
excerpt_visible_range,
),
))
}) })
.collect() .collect()
} }
@ -7256,7 +7273,7 @@ impl Editor {
fn on_buffer_event( fn on_buffer_event(
&mut self, &mut self,
_: ModelHandle<MultiBuffer>, multibuffer: ModelHandle<MultiBuffer>,
event: &multi_buffer::Event, event: &multi_buffer::Event,
cx: &mut ViewContext<Self>, cx: &mut ViewContext<Self>,
) { ) {
@ -7268,7 +7285,33 @@ impl Editor {
self.update_visible_copilot_suggestion(cx); self.update_visible_copilot_suggestion(cx);
} }
cx.emit(Event::BufferEdited); cx.emit(Event::BufferEdited);
self.refresh_inlays(InlayRefreshReason::ExcerptEdited, cx);
if let Some(project) = &self.project {
let project = project.read(cx);
let languages_affected = multibuffer
.read(cx)
.all_buffers()
.into_iter()
.filter_map(|buffer| {
let buffer = buffer.read(cx);
let language = buffer.language()?;
if project.is_local()
&& project.language_servers_for_buffer(buffer, cx).count() == 0
{
None
} else {
Some(language)
}
})
.cloned()
.collect::<HashSet<_>>();
if !languages_affected.is_empty() {
self.refresh_inlays(
InlayRefreshReason::BufferEdited(languages_affected),
cx,
);
}
}
} }
multi_buffer::Event::ExcerptsAdded { multi_buffer::Event::ExcerptsAdded {
buffer, buffer,

View File

@ -38,7 +38,7 @@ pub struct CachedExcerptHints {
#[derive(Debug, Clone, Copy)] #[derive(Debug, Clone, Copy)]
pub enum InvalidationStrategy { pub enum InvalidationStrategy {
RefreshRequested, RefreshRequested,
ExcerptEdited, BufferEdited,
None, None,
} }
@ -94,7 +94,7 @@ impl InvalidationStrategy {
fn should_invalidate(&self) -> bool { fn should_invalidate(&self) -> bool {
matches!( matches!(
self, self,
InvalidationStrategy::RefreshRequested | InvalidationStrategy::ExcerptEdited InvalidationStrategy::RefreshRequested | InvalidationStrategy::BufferEdited
) )
} }
} }
@ -197,7 +197,7 @@ impl InlayHintCache {
pub fn refresh_inlay_hints( pub fn refresh_inlay_hints(
&mut self, &mut self,
mut excerpts_to_query: HashMap<ExcerptId, (ModelHandle<Buffer>, Range<usize>)>, mut excerpts_to_query: HashMap<ExcerptId, (ModelHandle<Buffer>, Global, Range<usize>)>,
invalidate: InvalidationStrategy, invalidate: InvalidationStrategy,
cx: &mut ViewContext<Editor>, cx: &mut ViewContext<Editor>,
) { ) {
@ -342,30 +342,40 @@ impl InlayHintCache {
fn spawn_new_update_tasks( fn spawn_new_update_tasks(
editor: &mut Editor, editor: &mut Editor,
excerpts_to_query: HashMap<ExcerptId, (ModelHandle<Buffer>, Range<usize>)>, excerpts_to_query: HashMap<ExcerptId, (ModelHandle<Buffer>, Global, Range<usize>)>,
invalidate: InvalidationStrategy, invalidate: InvalidationStrategy,
update_cache_version: usize, update_cache_version: usize,
cx: &mut ViewContext<'_, '_, Editor>, cx: &mut ViewContext<'_, '_, Editor>,
) { ) {
let visible_hints = Arc::new(editor.visible_inlay_hints(cx)); let visible_hints = Arc::new(editor.visible_inlay_hints(cx));
for (excerpt_id, (buffer_handle, excerpt_visible_range)) in excerpts_to_query { for (excerpt_id, (buffer_handle, new_task_buffer_version, excerpt_visible_range)) in
if !excerpt_visible_range.is_empty() { excerpts_to_query
{
if excerpt_visible_range.is_empty() {
continue;
}
let buffer = buffer_handle.read(cx); let buffer = buffer_handle.read(cx);
let buffer_snapshot = buffer.snapshot(); let buffer_snapshot = buffer.snapshot();
if buffer_snapshot
.version()
.changed_since(&new_task_buffer_version)
{
continue;
}
let cached_excerpt_hints = editor.inlay_hint_cache.hints.get(&excerpt_id).cloned(); let cached_excerpt_hints = editor.inlay_hint_cache.hints.get(&excerpt_id).cloned();
if let Some(cached_excerpt_hints) = &cached_excerpt_hints { if let Some(cached_excerpt_hints) = &cached_excerpt_hints {
let new_task_buffer_version = buffer_snapshot.version();
let cached_excerpt_hints = cached_excerpt_hints.read(); let cached_excerpt_hints = cached_excerpt_hints.read();
let cached_buffer_version = &cached_excerpt_hints.buffer_version; let cached_buffer_version = &cached_excerpt_hints.buffer_version;
if cached_excerpt_hints.version > update_cache_version if cached_excerpt_hints.version > update_cache_version
|| cached_buffer_version.changed_since(new_task_buffer_version) || cached_buffer_version.changed_since(&new_task_buffer_version)
{ {
return; continue;
} }
if !new_task_buffer_version.changed_since(&cached_buffer_version) if !new_task_buffer_version.changed_since(&cached_buffer_version)
&& !matches!(invalidate, InvalidationStrategy::RefreshRequested) && !matches!(invalidate, InvalidationStrategy::RefreshRequested)
{ {
return; continue;
} }
}; };
@ -417,7 +427,7 @@ fn spawn_new_update_tasks(
match (update_task.invalidate, invalidate) { match (update_task.invalidate, invalidate) {
(_, InvalidationStrategy::None) => {} (_, InvalidationStrategy::None) => {}
( (
InvalidationStrategy::ExcerptEdited, InvalidationStrategy::BufferEdited,
InvalidationStrategy::RefreshRequested, InvalidationStrategy::RefreshRequested,
) if !update_task.task.is_running_rx.is_closed() => { ) if !update_task.task.is_running_rx.is_closed() => {
update_task.pending_refresh = Some(query); update_task.pending_refresh = Some(query);
@ -443,7 +453,6 @@ fn spawn_new_update_tasks(
} }
} }
} }
}
} }
fn new_update_task( fn new_update_task(
@ -961,6 +970,247 @@ mod tests {
}); });
} }
#[gpui::test]
async fn test_no_hint_updates_for_unrelated_language_files(cx: &mut gpui::TestAppContext) {
let allowed_hint_kinds = HashSet::from_iter([None, Some(InlayHintKind::Type)]);
init_test(cx, |settings| {
settings.defaults.inlay_hints = Some(InlayHintSettings {
enabled: true,
show_type_hints: allowed_hint_kinds.contains(&Some(InlayHintKind::Type)),
show_parameter_hints: allowed_hint_kinds.contains(&Some(InlayHintKind::Parameter)),
show_other_hints: allowed_hint_kinds.contains(&None),
})
});
let fs = FakeFs::new(cx.background());
fs.insert_tree(
"/a",
json!({
"main.rs": "fn main() { a } // and some long comment to ensure inlays are not trimmed out",
"other.md": "Test md file with some text",
}),
)
.await;
let project = Project::test(fs, ["/a".as_ref()], cx).await;
let (_, workspace) = cx.add_window(|cx| Workspace::test_new(project.clone(), cx));
let worktree_id = workspace.update(cx, |workspace, cx| {
workspace.project().read_with(cx, |project, cx| {
project.worktrees(cx).next().unwrap().read(cx).id()
})
});
let mut rs_fake_servers = None;
let mut md_fake_servers = None;
for (name, path_suffix) in [("Rust", "rs"), ("Markdown", "md")] {
let mut language = Language::new(
LanguageConfig {
name: name.into(),
path_suffixes: vec![path_suffix.to_string()],
..Default::default()
},
Some(tree_sitter_rust::language()),
);
let fake_servers = language
.set_fake_lsp_adapter(Arc::new(FakeLspAdapter {
name,
capabilities: lsp::ServerCapabilities {
inlay_hint_provider: Some(lsp::OneOf::Left(true)),
..Default::default()
},
..Default::default()
}))
.await;
match name {
"Rust" => rs_fake_servers = Some(fake_servers),
"Markdown" => md_fake_servers = Some(fake_servers),
_ => unreachable!(),
}
project.update(cx, |project, _| {
project.languages().add(Arc::new(language));
});
}
let _rs_buffer = project
.update(cx, |project, cx| {
project.open_local_buffer("/a/main.rs", cx)
})
.await
.unwrap();
cx.foreground().run_until_parked();
cx.foreground().start_waiting();
let rs_fake_server = rs_fake_servers.unwrap().next().await.unwrap();
let rs_editor = workspace
.update(cx, |workspace, cx| {
workspace.open_path((worktree_id, "main.rs"), None, true, cx)
})
.await
.unwrap()
.downcast::<Editor>()
.unwrap();
let rs_lsp_request_count = Arc::new(AtomicU32::new(0));
rs_fake_server
.handle_request::<lsp::request::InlayHintRequest, _, _>(move |params, _| {
let task_lsp_request_count = Arc::clone(&rs_lsp_request_count);
async move {
assert_eq!(
params.text_document.uri,
lsp::Url::from_file_path("/a/main.rs").unwrap(),
);
let i = Arc::clone(&task_lsp_request_count).fetch_add(1, Ordering::SeqCst);
Ok(Some(vec![lsp::InlayHint {
position: lsp::Position::new(0, i),
label: lsp::InlayHintLabel::String(i.to_string()),
kind: None,
text_edits: None,
tooltip: None,
padding_left: None,
padding_right: None,
data: None,
}]))
}
})
.next()
.await;
cx.foreground().run_until_parked();
rs_editor.update(cx, |editor, cx| {
let expected_layers = vec!["0".to_string()];
assert_eq!(
expected_layers,
cached_hint_labels(editor),
"Should get its first hints when opening the editor"
);
assert_eq!(expected_layers, visible_hint_labels(editor, cx));
let inlay_cache = editor.inlay_hint_cache();
assert_eq!(
inlay_cache.allowed_hint_kinds, allowed_hint_kinds,
"Cache should use editor settings to get the allowed hint kinds"
);
assert_eq!(
inlay_cache.version, 1,
"Rust editor update the cache version after every cache/view change"
);
});
cx.foreground().run_until_parked();
let _md_buffer = project
.update(cx, |project, cx| {
project.open_local_buffer("/a/other.md", cx)
})
.await
.unwrap();
cx.foreground().run_until_parked();
cx.foreground().start_waiting();
let md_fake_server = md_fake_servers.unwrap().next().await.unwrap();
let md_editor = workspace
.update(cx, |workspace, cx| {
workspace.open_path((worktree_id, "other.md"), None, true, cx)
})
.await
.unwrap()
.downcast::<Editor>()
.unwrap();
let md_lsp_request_count = Arc::new(AtomicU32::new(0));
md_fake_server
.handle_request::<lsp::request::InlayHintRequest, _, _>(move |params, _| {
let task_lsp_request_count = Arc::clone(&md_lsp_request_count);
async move {
assert_eq!(
params.text_document.uri,
lsp::Url::from_file_path("/a/other.md").unwrap(),
);
let i = Arc::clone(&task_lsp_request_count).fetch_add(1, Ordering::SeqCst);
Ok(Some(vec![lsp::InlayHint {
position: lsp::Position::new(0, i),
label: lsp::InlayHintLabel::String(i.to_string()),
kind: None,
text_edits: None,
tooltip: None,
padding_left: None,
padding_right: None,
data: None,
}]))
}
})
.next()
.await;
cx.foreground().run_until_parked();
md_editor.update(cx, |editor, cx| {
let expected_layers = vec!["0".to_string()];
assert_eq!(
expected_layers,
cached_hint_labels(editor),
"Markdown editor should have a separate verison, repeating Rust editor rules"
);
assert_eq!(expected_layers, visible_hint_labels(editor, cx));
let inlay_cache = editor.inlay_hint_cache();
assert_eq!(inlay_cache.allowed_hint_kinds, allowed_hint_kinds);
assert_eq!(inlay_cache.version, 1);
});
rs_editor.update(cx, |editor, cx| {
editor.change_selections(None, cx, |s| s.select_ranges([13..13]));
editor.handle_input("some rs change", cx);
});
cx.foreground().run_until_parked();
rs_editor.update(cx, |editor, cx| {
let expected_layers = vec!["1".to_string()];
assert_eq!(
expected_layers,
cached_hint_labels(editor),
"Rust inlay cache should change after the edit"
);
assert_eq!(expected_layers, visible_hint_labels(editor, cx));
let inlay_cache = editor.inlay_hint_cache();
assert_eq!(inlay_cache.allowed_hint_kinds, allowed_hint_kinds);
assert_eq!(
inlay_cache.version, 2,
"Every time hint cache changes, cache version should be incremented"
);
});
md_editor.update(cx, |editor, cx| {
let expected_layers = vec!["0".to_string()];
assert_eq!(
expected_layers,
cached_hint_labels(editor),
"Markdown editor should not be affected by Rust editor changes"
);
assert_eq!(expected_layers, visible_hint_labels(editor, cx));
let inlay_cache = editor.inlay_hint_cache();
assert_eq!(inlay_cache.allowed_hint_kinds, allowed_hint_kinds);
assert_eq!(inlay_cache.version, 1);
});
md_editor.update(cx, |editor, cx| {
editor.change_selections(None, cx, |s| s.select_ranges([13..13]));
editor.handle_input("some md change", cx);
});
cx.foreground().run_until_parked();
md_editor.update(cx, |editor, cx| {
let expected_layers = vec!["1".to_string()];
assert_eq!(
expected_layers,
cached_hint_labels(editor),
"Rust editor should not be affected by Markdown editor changes"
);
assert_eq!(expected_layers, visible_hint_labels(editor, cx));
let inlay_cache = editor.inlay_hint_cache();
assert_eq!(inlay_cache.allowed_hint_kinds, allowed_hint_kinds);
assert_eq!(inlay_cache.version, 2);
});
rs_editor.update(cx, |editor, cx| {
let expected_layers = vec!["1".to_string()];
assert_eq!(
expected_layers,
cached_hint_labels(editor),
"Markdown editor should also change independently"
);
assert_eq!(expected_layers, visible_hint_labels(editor, cx));
let inlay_cache = editor.inlay_hint_cache();
assert_eq!(inlay_cache.allowed_hint_kinds, allowed_hint_kinds);
assert_eq!(inlay_cache.version, 2);
});
}
#[gpui::test] #[gpui::test]
async fn test_hint_setting_changes(cx: &mut gpui::TestAppContext) { async fn test_hint_setting_changes(cx: &mut gpui::TestAppContext) {
let allowed_hint_kinds = HashSet::from_iter([None, Some(InlayHintKind::Type)]); let allowed_hint_kinds = HashSet::from_iter([None, Some(InlayHintKind::Type)]);

View File

@ -2489,7 +2489,12 @@ impl ToOffset for Point {
impl ToOffset for usize { impl ToOffset for usize {
fn to_offset(&self, snapshot: &BufferSnapshot) -> usize { fn to_offset(&self, snapshot: &BufferSnapshot) -> usize {
assert!(*self <= snapshot.len(), "offset {self} is out of range"); assert!(
*self <= snapshot.len(),
"offset {} is out of range, max allowed is {}",
self,
snapshot.len()
);
*self *self
} }
} }