Improve inlay hints management (#2897)

Apply a rate limiter to hints scrolling, properly invalidate hint task
cached ranges, properly react to multibuffer events, properly highlight
hints and introduce basic debug logging for hints.

Release Notes:

- N/A
This commit is contained in:
Kirill Bulatov 2023-08-26 16:50:12 +03:00 committed by GitHub
commit 851d74d5e1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 307 additions and 79 deletions

View File

@ -1251,6 +1251,19 @@ enum InlayHintRefreshReason {
NewLinesShown,
BufferEdited(HashSet<Arc<Language>>),
RefreshRequested,
ExcerptsRemoved(Vec<ExcerptId>),
}
impl InlayHintRefreshReason {
fn description(&self) -> &'static str {
match self {
Self::Toggle(_) => "toggle",
Self::SettingsChange(_) => "settings change",
Self::NewLinesShown => "new lines shown",
Self::BufferEdited(_) => "buffer edited",
Self::RefreshRequested => "refresh requested",
Self::ExcerptsRemoved(_) => "excerpts removed",
}
}
}
impl Editor {
@ -2741,6 +2754,7 @@ impl Editor {
return;
}
let reason_description = reason.description();
let (invalidate_cache, required_languages) = match reason {
InlayHintRefreshReason::Toggle(enabled) => {
self.inlay_hint_cache.enabled = enabled;
@ -2777,6 +2791,14 @@ impl Editor {
ControlFlow::Continue(()) => (InvalidationStrategy::RefreshRequested, None),
}
}
InlayHintRefreshReason::ExcerptsRemoved(excerpts_removed) => {
let InlaySplice {
to_remove,
to_insert,
} = self.inlay_hint_cache.remove_excerpts(excerpts_removed);
self.splice_inlay_hints(to_remove, to_insert, cx);
return;
}
InlayHintRefreshReason::NewLinesShown => (InvalidationStrategy::None, None),
InlayHintRefreshReason::BufferEdited(buffer_languages) => {
(InvalidationStrategy::BufferEdited, Some(buffer_languages))
@ -2790,6 +2812,7 @@ impl Editor {
to_remove,
to_insert,
}) = self.inlay_hint_cache.spawn_hint_refresh(
reason_description,
self.excerpt_visible_offsets(required_languages.as_ref(), cx),
invalidate_cache,
cx,
@ -7883,7 +7906,9 @@ impl Editor {
cx: &mut ViewContext<Self>,
) {
match event {
multi_buffer::Event::Edited => {
multi_buffer::Event::Edited {
sigleton_buffer_edited,
} => {
self.refresh_active_diagnostics(cx);
self.refresh_code_actions(cx);
if self.has_active_copilot_suggestion(cx) {
@ -7891,30 +7916,32 @@ impl Editor {
}
cx.emit(Event::BufferEdited);
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_inlay_hints(
InlayHintRefreshReason::BufferEdited(languages_affected),
cx,
);
if *sigleton_buffer_edited {
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_inlay_hints(
InlayHintRefreshReason::BufferEdited(languages_affected),
cx,
);
}
}
}
}
@ -7922,12 +7949,16 @@ impl Editor {
buffer,
predecessor,
excerpts,
} => cx.emit(Event::ExcerptsAdded {
buffer: buffer.clone(),
predecessor: *predecessor,
excerpts: excerpts.clone(),
}),
} => {
cx.emit(Event::ExcerptsAdded {
buffer: buffer.clone(),
predecessor: *predecessor,
excerpts: excerpts.clone(),
});
self.refresh_inlay_hints(InlayHintRefreshReason::NewLinesShown, cx);
}
multi_buffer::Event::ExcerptsRemoved { ids } => {
self.refresh_inlay_hints(InlayHintRefreshReason::ExcerptsRemoved(ids.clone()), cx);
cx.emit(Event::ExcerptsRemoved { ids: ids.clone() })
}
multi_buffer::Event::Reparsed => cx.emit(Event::Reparsed),

View File

@ -65,8 +65,7 @@ pub fn find_hovered_hint_part(
if hovered_offset >= hint_range.start && hovered_offset <= hint_range.end {
let mut hovered_character = (hovered_offset - hint_range.start).0;
let mut part_start = hint_range.start;
let last_label_part_index = label_parts.len() - 1;
for (i, part) in label_parts.into_iter().enumerate() {
for part in label_parts {
let part_len = part.value.chars().count();
if hovered_character >= part_len {
hovered_character -= part_len;
@ -77,8 +76,9 @@ pub fn find_hovered_hint_part(
part_start.0 += 1;
part_end.0 += 1;
}
if padding_right && i == last_label_part_index {
part_end.0 -= 1;
if padding_right {
part_start.0 += 1;
part_end.0 += 1;
}
return Some((part, part_start..part_end));
}

View File

@ -13,14 +13,14 @@ use clock::Global;
use futures::future;
use gpui::{ModelContext, ModelHandle, Task, ViewContext};
use language::{language_settings::InlayHintKind, Buffer, BufferSnapshot};
use log::error;
use parking_lot::RwLock;
use project::{InlayHint, ResolveState};
use collections::{hash_map, HashMap, HashSet};
use language::language_settings::InlayHintSettings;
use smol::lock::Semaphore;
use sum_tree::Bias;
use text::ToOffset;
use text::{ToOffset, ToPoint};
use util::post_inc;
pub struct InlayHintCache {
@ -29,6 +29,7 @@ pub struct InlayHintCache {
version: usize,
pub(super) enabled: bool,
update_tasks: HashMap<ExcerptId, TasksForRanges>,
lsp_request_limiter: Arc<Semaphore>,
}
#[derive(Debug)]
@ -72,6 +73,7 @@ struct ExcerptQuery {
excerpt_id: ExcerptId,
cache_version: usize,
invalidate: InvalidationStrategy,
reason: &'static str,
}
impl InvalidationStrategy {
@ -108,17 +110,23 @@ impl TasksForRanges {
updated_ranges.before_visible = updated_ranges
.before_visible
.into_iter()
.flat_map(|query_range| self.remove_cached_ranges(buffer_snapshot, query_range))
.flat_map(|query_range| {
self.remove_cached_ranges_from_query(buffer_snapshot, query_range)
})
.collect();
updated_ranges.visible = updated_ranges
.visible
.into_iter()
.flat_map(|query_range| self.remove_cached_ranges(buffer_snapshot, query_range))
.flat_map(|query_range| {
self.remove_cached_ranges_from_query(buffer_snapshot, query_range)
})
.collect();
updated_ranges.after_visible = updated_ranges
.after_visible
.into_iter()
.flat_map(|query_range| self.remove_cached_ranges(buffer_snapshot, query_range))
.flat_map(|query_range| {
self.remove_cached_ranges_from_query(buffer_snapshot, query_range)
})
.collect();
updated_ranges
}
@ -134,7 +142,7 @@ impl TasksForRanges {
}
}
fn remove_cached_ranges(
fn remove_cached_ranges_from_query(
&mut self,
buffer_snapshot: &BufferSnapshot,
query_range: Range<language::Anchor>,
@ -196,6 +204,52 @@ impl TasksForRanges {
ranges_to_query
}
fn remove_from_cached_ranges(
&mut self,
buffer: &BufferSnapshot,
range_to_remove: &Range<language::Anchor>,
) {
self.sorted_ranges = self
.sorted_ranges
.drain(..)
.filter_map(|mut cached_range| {
if cached_range.start.cmp(&range_to_remove.end, buffer).is_gt()
|| cached_range.end.cmp(&range_to_remove.start, buffer).is_lt()
{
Some(vec![cached_range])
} else if cached_range
.start
.cmp(&range_to_remove.start, buffer)
.is_ge()
&& cached_range.end.cmp(&range_to_remove.end, buffer).is_le()
{
None
} else if range_to_remove
.start
.cmp(&cached_range.start, buffer)
.is_ge()
&& range_to_remove.end.cmp(&cached_range.end, buffer).is_le()
{
Some(vec![
cached_range.start..range_to_remove.start,
range_to_remove.end..cached_range.end,
])
} else if cached_range
.start
.cmp(&range_to_remove.start, buffer)
.is_ge()
{
cached_range.start = range_to_remove.end;
Some(vec![cached_range])
} else {
cached_range.end = range_to_remove.start;
Some(vec![cached_range])
}
})
.flatten()
.collect();
}
}
impl InlayHintCache {
@ -206,6 +260,7 @@ impl InlayHintCache {
hints: HashMap::default(),
update_tasks: HashMap::default(),
version: 0,
lsp_request_limiter: Arc::new(Semaphore::new(MAX_CONCURRENT_LSP_REQUESTS)),
}
}
@ -262,6 +317,7 @@ impl InlayHintCache {
pub fn spawn_hint_refresh(
&mut self,
reason: &'static str,
excerpts_to_query: HashMap<ExcerptId, (ModelHandle<Buffer>, Global, Range<usize>)>,
invalidate: InvalidationStrategy,
cx: &mut ViewContext<Editor>,
@ -290,7 +346,14 @@ impl InlayHintCache {
cx.spawn(|editor, mut cx| async move {
editor
.update(&mut cx, |editor, cx| {
spawn_new_update_tasks(editor, excerpts_to_query, invalidate, cache_version, cx)
spawn_new_update_tasks(
editor,
reason,
excerpts_to_query,
invalidate,
cache_version,
cx,
)
})
.ok();
})
@ -411,6 +474,24 @@ impl InlayHintCache {
}
}
pub fn remove_excerpts(&mut self, excerpts_removed: Vec<ExcerptId>) -> InlaySplice {
let mut to_remove = Vec::new();
for excerpt_to_remove in excerpts_removed {
self.update_tasks.remove(&excerpt_to_remove);
if let Some(cached_hints) = self.hints.remove(&excerpt_to_remove) {
let cached_hints = cached_hints.read();
to_remove.extend(cached_hints.hints.iter().map(|(id, _)| *id));
}
}
if !to_remove.is_empty() {
self.version += 1;
}
InlaySplice {
to_remove,
to_insert: Vec::new(),
}
}
pub fn clear(&mut self) {
self.version += 1;
self.update_tasks.clear();
@ -513,6 +594,7 @@ impl InlayHintCache {
fn spawn_new_update_tasks(
editor: &mut Editor,
reason: &'static str,
excerpts_to_query: HashMap<ExcerptId, (ModelHandle<Buffer>, Global, Range<usize>)>,
invalidate: InvalidationStrategy,
update_cache_version: usize,
@ -567,6 +649,7 @@ fn spawn_new_update_tasks(
excerpt_id,
cache_version: update_cache_version,
invalidate,
reason,
};
let new_update_task = |query_ranges| {
@ -577,6 +660,7 @@ fn spawn_new_update_tasks(
buffer_snapshot.clone(),
Arc::clone(&visible_hints),
cached_excerpt_hints,
Arc::clone(&editor.inlay_hint_cache.lsp_request_limiter),
cx,
)
};
@ -681,6 +765,7 @@ fn determine_query_ranges(
})
}
const MAX_CONCURRENT_LSP_REQUESTS: usize = 5;
const INVISIBLE_RANGES_HINTS_REQUEST_DELAY_MILLIS: u64 = 400;
fn new_update_task(
@ -690,9 +775,11 @@ fn new_update_task(
buffer_snapshot: BufferSnapshot,
visible_hints: Arc<Vec<Inlay>>,
cached_excerpt_hints: Option<Arc<RwLock<CachedExcerptHints>>>,
lsp_request_limiter: Arc<Semaphore>,
cx: &mut ViewContext<'_, '_, Editor>,
) -> Task<()> {
cx.spawn(|editor, cx| async move {
cx.spawn(|editor, mut cx| async move {
let closure_cx = cx.clone();
let fetch_and_update_hints = |invalidate, range| {
fetch_and_update_hints(
editor.clone(),
@ -703,37 +790,63 @@ fn new_update_task(
query,
invalidate,
range,
cx.clone(),
Arc::clone(&lsp_request_limiter),
closure_cx.clone(),
)
};
let visible_range_update_results =
future::join_all(query_ranges.visible.into_iter().map(|visible_range| {
fetch_and_update_hints(query.invalidate.should_invalidate(), visible_range)
}))
.await;
for result in visible_range_update_results {
let visible_range_update_results = future::join_all(query_ranges.visible.into_iter().map(
|visible_range| async move {
(
visible_range.clone(),
fetch_and_update_hints(query.invalidate.should_invalidate(), visible_range)
.await,
)
},
))
.await;
let hint_delay = cx.background().timer(Duration::from_millis(
INVISIBLE_RANGES_HINTS_REQUEST_DELAY_MILLIS,
));
let mut query_range_failed = |range: &Range<language::Anchor>, e: anyhow::Error| {
log::error!("inlay hint update task for range {range:?} failed: {e:#}");
editor
.update(&mut cx, |editor, _| {
if let Some(task_ranges) = editor
.inlay_hint_cache
.update_tasks
.get_mut(&query.excerpt_id)
{
task_ranges.remove_from_cached_ranges(&buffer_snapshot, &range);
}
})
.ok()
};
for (range, result) in visible_range_update_results {
if let Err(e) = result {
error!("visible range inlay hint update task failed: {e:#}");
query_range_failed(&range, e);
}
}
cx.background()
.timer(Duration::from_millis(
INVISIBLE_RANGES_HINTS_REQUEST_DELAY_MILLIS,
))
.await;
hint_delay.await;
let invisible_range_update_results = future::join_all(
query_ranges
.before_visible
.into_iter()
.chain(query_ranges.after_visible.into_iter())
.map(|invisible_range| fetch_and_update_hints(false, invisible_range)),
.map(|invisible_range| async move {
(
invisible_range.clone(),
fetch_and_update_hints(false, invisible_range).await,
)
}),
)
.await;
for result in invisible_range_update_results {
for (range, result) in invisible_range_update_results {
if let Err(e) = result {
error!("invisible range inlay hint update task failed: {e:#}");
query_range_failed(&range, e);
}
}
})
@ -748,10 +861,51 @@ async fn fetch_and_update_hints(
query: ExcerptQuery,
invalidate: bool,
fetch_range: Range<language::Anchor>,
lsp_request_limiter: Arc<Semaphore>,
mut cx: gpui::AsyncAppContext,
) -> anyhow::Result<()> {
let (lsp_request_guard, got_throttled) = if query.invalidate.should_invalidate() {
(None, false)
} else {
match lsp_request_limiter.try_acquire() {
Some(guard) => (Some(guard), false),
None => (Some(lsp_request_limiter.acquire().await), true),
}
};
let fetch_range_to_log =
fetch_range.start.to_point(&buffer_snapshot)..fetch_range.end.to_point(&buffer_snapshot);
let inlay_hints_fetch_task = editor
.update(&mut cx, |editor, cx| {
if got_throttled {
if let Some((_, _, current_visible_range)) = editor
.excerpt_visible_offsets(None, cx)
.remove(&query.excerpt_id)
{
let visible_offset_length = current_visible_range.len();
let double_visible_range = current_visible_range
.start
.saturating_sub(visible_offset_length)
..current_visible_range
.end
.saturating_add(visible_offset_length)
.min(buffer_snapshot.len());
if !double_visible_range
.contains(&fetch_range.start.to_offset(&buffer_snapshot))
&& !double_visible_range
.contains(&fetch_range.end.to_offset(&buffer_snapshot))
{
log::trace!("Fetching inlay hints for range {fetch_range_to_log:?} got throttled and fell off the current visible range, skipping.");
if let Some(task_ranges) = editor
.inlay_hint_cache
.update_tasks
.get_mut(&query.excerpt_id)
{
task_ranges.remove_from_cached_ranges(&buffer_snapshot, &fetch_range);
}
return None;
}
}
}
editor
.buffer()
.read(cx)
@ -766,9 +920,26 @@ async fn fetch_and_update_hints(
.ok()
.flatten();
let new_hints = match inlay_hints_fetch_task {
Some(task) => task.await.context("inlay hint fetch task")?,
Some(fetch_task) => {
log::debug!(
"Fetching inlay hints for range {fetch_range_to_log:?}, reason: {query_reason}, invalidate: {invalidate}",
query_reason = query.reason,
);
log::trace!(
"Currently visible hints: {visible_hints:?}, cached hints present: {}",
cached_excerpt_hints.is_some(),
);
fetch_task.await.context("inlay hint fetch task")?
}
None => return Ok(()),
};
drop(lsp_request_guard);
log::debug!(
"Fetched {} hints for range {fetch_range_to_log:?}",
new_hints.len()
);
log::trace!("Fetched hints: {new_hints:?}");
let background_task_buffer_snapshot = buffer_snapshot.clone();
let backround_fetch_range = fetch_range.clone();
let new_update = cx
@ -786,6 +957,13 @@ async fn fetch_and_update_hints(
})
.await;
if let Some(new_update) = new_update {
log::info!(
"Applying update for range {fetch_range_to_log:?}: remove from editor: {}, remove from cache: {}, add to cache: {}",
new_update.remove_from_visible.len(),
new_update.remove_from_cache.len(),
new_update.add_to_cache.len()
);
log::trace!("New update: {new_update:?}");
editor
.update(&mut cx, |editor, cx| {
apply_hint_update(
@ -2796,7 +2974,7 @@ all hints should be invalidated and requeried for all of its visible excerpts"
);
assert_eq!(
editor.inlay_hint_cache().version,
2,
3,
"Excerpt removal should trigger a cache update"
);
});
@ -2824,7 +3002,7 @@ all hints should be invalidated and requeried for all of its visible excerpts"
);
assert_eq!(
editor.inlay_hint_cache().version,
3,
4,
"Settings change should trigger a cache update"
);
});

View File

@ -173,6 +173,8 @@ pub fn update_inlay_link_and_hover_points(
} else {
None
};
let mut go_to_definition_updated = false;
let mut hover_updated = false;
if let Some(hovered_offset) = hovered_offset {
let buffer_snapshot = editor.buffer().read(cx).snapshot(cx);
let previous_valid_anchor = buffer_snapshot.anchor_at(
@ -183,9 +185,6 @@ pub fn update_inlay_link_and_hover_points(
point_for_position.next_valid.to_point(snapshot),
Bias::Right,
);
let mut go_to_definition_updated = false;
let mut hover_updated = false;
if let Some(hovered_hint) = editor
.visible_inlay_hints(cx)
.into_iter()
@ -324,13 +323,13 @@ pub fn update_inlay_link_and_hover_points(
}
}
}
}
if !go_to_definition_updated {
update_go_to_definition_link(editor, None, cmd_held, shift_held, cx);
}
if !hover_updated {
hover_popover::hover_at(editor, None, cx);
}
if !go_to_definition_updated {
update_go_to_definition_link(editor, None, cmd_held, shift_held, cx);
}
if !hover_updated {
hover_popover::hover_at(editor, None, cx);
}
}

View File

@ -67,7 +67,9 @@ pub enum Event {
ExcerptsEdited {
ids: Vec<ExcerptId>,
},
Edited,
Edited {
sigleton_buffer_edited: bool,
},
Reloaded,
DiffBaseChanged,
LanguageChanged,
@ -1022,7 +1024,9 @@ impl MultiBuffer {
old: edit_start..edit_start,
new: edit_start..edit_end,
}]);
cx.emit(Event::Edited);
cx.emit(Event::Edited {
sigleton_buffer_edited: false,
});
cx.emit(Event::ExcerptsAdded {
buffer,
predecessor: prev_excerpt_id,
@ -1046,7 +1050,9 @@ impl MultiBuffer {
old: 0..prev_len,
new: 0..0,
}]);
cx.emit(Event::Edited);
cx.emit(Event::Edited {
sigleton_buffer_edited: false,
});
cx.emit(Event::ExcerptsRemoved { ids });
cx.notify();
}
@ -1254,7 +1260,9 @@ impl MultiBuffer {
}
self.subscriptions.publish_mut(edits);
cx.emit(Event::Edited);
cx.emit(Event::Edited {
sigleton_buffer_edited: false,
});
cx.emit(Event::ExcerptsRemoved { ids });
cx.notify();
}
@ -1315,7 +1323,9 @@ impl MultiBuffer {
cx: &mut ModelContext<Self>,
) {
cx.emit(match event {
language::Event::Edited => Event::Edited,
language::Event::Edited => Event::Edited {
sigleton_buffer_edited: true,
},
language::Event::DirtyChanged => Event::DirtyChanged,
language::Event::Saved => Event::Saved,
language::Event::FileHandleChanged => Event::FileHandleChanged,
@ -4078,7 +4088,7 @@ mod tests {
multibuffer.update(cx, |_, cx| {
let events = events.clone();
cx.subscribe(&multibuffer, move |_, _, event, _| {
if let Event::Edited = event {
if let Event::Edited { .. } = event {
events.borrow_mut().push(event.clone())
}
})
@ -4133,7 +4143,17 @@ mod tests {
// Adding excerpts emits an edited event.
assert_eq!(
events.borrow().as_slice(),
&[Event::Edited, Event::Edited, Event::Edited]
&[
Event::Edited {
sigleton_buffer_edited: false
},
Event::Edited {
sigleton_buffer_edited: false
},
Event::Edited {
sigleton_buffer_edited: false
}
]
);
let snapshot = multibuffer.read(cx).snapshot(cx);
@ -4312,7 +4332,7 @@ mod tests {
excerpts,
} => follower.insert_excerpts_with_ids_after(predecessor, buffer, excerpts, cx),
Event::ExcerptsRemoved { ids } => follower.remove_excerpts(ids, cx),
Event::Edited => {
Event::Edited { .. } => {
*follower_edit_event_count.borrow_mut() += 1;
}
_ => {}