Query less inlay hints (#2842)

Kirill Bulatov created

Part of
https://linear.app/zed-industries/issue/Z-2750/investigate-performance-of-collaborating-on-large-files-with-inlay

Instead of querying the entire file for hints, query visible editor(s)
range + the areas above and below, of the same height.
Non-invalidating future queries (e.g. scrolling) query only missing
parts of the ranges.

Release Notes:

- Improved LSP resource usage by querying less hints for big files

Change summary

crates/collab/src/tests/integration_tests.rs |  42 
crates/editor/src/editor.rs                  |   2 
crates/editor/src/inlay_hint_cache.rs        | 892 +++++++++++----------
crates/editor/src/scroll.rs                  |   6 
4 files changed, 499 insertions(+), 443 deletions(-)

Detailed changes

crates/collab/src/tests/integration_tests.rs 🔗

@@ -7953,7 +7953,8 @@ async fn test_mutual_editor_inlay_hint_cache_update(
         );
         let inlay_cache = editor.inlay_hint_cache();
         assert_eq!(
-            inlay_cache.version, edits_made,
+            inlay_cache.version(),
+            edits_made,
             "Host editor update the cache version after every cache/view change",
         );
     });
@@ -7976,7 +7977,8 @@ async fn test_mutual_editor_inlay_hint_cache_update(
         );
         let inlay_cache = editor.inlay_hint_cache();
         assert_eq!(
-            inlay_cache.version, edits_made,
+            inlay_cache.version(),
+            edits_made,
             "Guest editor update the cache version after every cache/view change"
         );
     });
@@ -7996,7 +7998,7 @@ async fn test_mutual_editor_inlay_hint_cache_update(
             "Host should get hints from the 1st edit and 1st LSP query"
         );
         let inlay_cache = editor.inlay_hint_cache();
-        assert_eq!(inlay_cache.version, edits_made);
+        assert_eq!(inlay_cache.version(), edits_made);
     });
     editor_b.update(cx_b, |editor, _| {
         assert_eq!(
@@ -8010,7 +8012,7 @@ async fn test_mutual_editor_inlay_hint_cache_update(
             "Guest should get hints the 1st edit and 2nd LSP query"
         );
         let inlay_cache = editor.inlay_hint_cache();
-        assert_eq!(inlay_cache.version, edits_made);
+        assert_eq!(inlay_cache.version(), edits_made);
     });
 
     editor_a.update(cx_a, |editor, cx| {
@@ -8035,7 +8037,7 @@ async fn test_mutual_editor_inlay_hint_cache_update(
 4th query was made by guest (but not applied) due to cache invalidation logic"
         );
         let inlay_cache = editor.inlay_hint_cache();
-        assert_eq!(inlay_cache.version, edits_made);
+        assert_eq!(inlay_cache.version(), edits_made);
     });
     editor_b.update(cx_b, |editor, _| {
         assert_eq!(
@@ -8051,7 +8053,7 @@ async fn test_mutual_editor_inlay_hint_cache_update(
             "Guest should get hints from 3rd edit, 6th LSP query"
         );
         let inlay_cache = editor.inlay_hint_cache();
-        assert_eq!(inlay_cache.version, edits_made);
+        assert_eq!(inlay_cache.version(), edits_made);
     });
 
     fake_language_server
@@ -8077,7 +8079,8 @@ async fn test_mutual_editor_inlay_hint_cache_update(
         );
         let inlay_cache = editor.inlay_hint_cache();
         assert_eq!(
-            inlay_cache.version, edits_made,
+            inlay_cache.version(),
+            edits_made,
             "Host should accepted all edits and bump its cache version every time"
         );
     });
@@ -8098,7 +8101,7 @@ async fn test_mutual_editor_inlay_hint_cache_update(
         );
         let inlay_cache = editor.inlay_hint_cache();
         assert_eq!(
-            inlay_cache.version,
+            inlay_cache.version(),
             edits_made,
             "Guest should accepted all edits and bump its cache version every time"
         );
@@ -8264,7 +8267,8 @@ async fn test_inlay_hint_refresh_is_forwarded(
         );
         let inlay_cache = editor.inlay_hint_cache();
         assert_eq!(
-            inlay_cache.version, 0,
+            inlay_cache.version(),
+            0,
             "Host should not increment its cache version due to no changes",
         );
     });
@@ -8279,7 +8283,8 @@ async fn test_inlay_hint_refresh_is_forwarded(
         );
         let inlay_cache = editor.inlay_hint_cache();
         assert_eq!(
-            inlay_cache.version, edits_made,
+            inlay_cache.version(),
+            edits_made,
             "Guest editor update the cache version after every cache/view change"
         );
     });
@@ -8296,7 +8301,8 @@ async fn test_inlay_hint_refresh_is_forwarded(
         );
         let inlay_cache = editor.inlay_hint_cache();
         assert_eq!(
-            inlay_cache.version, 0,
+            inlay_cache.version(),
+            0,
             "Host should not increment its cache version due to no changes",
         );
     });
@@ -8311,7 +8317,8 @@ async fn test_inlay_hint_refresh_is_forwarded(
         );
         let inlay_cache = editor.inlay_hint_cache();
         assert_eq!(
-            inlay_cache.version, edits_made,
+            inlay_cache.version(),
+            edits_made,
             "Guest should accepted all edits and bump its cache version every time"
         );
     });
@@ -8343,13 +8350,10 @@ fn room_participants(room: &ModelHandle<Room>, cx: &mut TestAppContext) -> RoomP
 
 fn extract_hint_labels(editor: &Editor) -> Vec<String> {
     let mut labels = Vec::new();
-    for (_, excerpt_hints) in &editor.inlay_hint_cache().hints {
-        let excerpt_hints = excerpt_hints.read();
-        for (_, inlay) in excerpt_hints.hints.iter() {
-            match &inlay.label {
-                project::InlayHintLabel::String(s) => labels.push(s.to_string()),
-                _ => unreachable!(),
-            }
+    for hint in editor.inlay_hint_cache().hints() {
+        match hint.label {
+            project::InlayHintLabel::String(s) => labels.push(s),
+            _ => unreachable!(),
         }
     }
     labels

crates/editor/src/editor.rs 🔗

@@ -2723,7 +2723,7 @@ impl Editor {
             .collect()
     }
 
-    fn excerpt_visible_offsets(
+    pub fn excerpt_visible_offsets(
         &self,
         restrict_to_languages: Option<&HashSet<Arc<Language>>>,
         cx: &mut ViewContext<'_, '_, Editor>,

crates/editor/src/inlay_hint_cache.rs 🔗

@@ -9,7 +9,7 @@ use crate::{
 };
 use anyhow::Context;
 use clock::Global;
-use gpui::{ModelHandle, Task, ViewContext};
+use gpui::{ModelContext, ModelHandle, Task, ViewContext};
 use language::{language_settings::InlayHintKind, Buffer, BufferSnapshot};
 use log::error;
 use parking_lot::RwLock;
@@ -17,14 +17,21 @@ use project::InlayHint;
 
 use collections::{hash_map, HashMap, HashSet};
 use language::language_settings::InlayHintSettings;
+use sum_tree::Bias;
 use util::post_inc;
 
 pub struct InlayHintCache {
-    pub hints: HashMap<ExcerptId, Arc<RwLock<CachedExcerptHints>>>,
-    pub allowed_hint_kinds: HashSet<Option<InlayHintKind>>,
-    pub version: usize,
-    pub enabled: bool,
-    update_tasks: HashMap<ExcerptId, UpdateTask>,
+    hints: HashMap<ExcerptId, Arc<RwLock<CachedExcerptHints>>>,
+    allowed_hint_kinds: HashSet<Option<InlayHintKind>>,
+    version: usize,
+    enabled: bool,
+    update_tasks: HashMap<ExcerptId, TasksForRanges>,
+}
+
+#[derive(Debug)]
+struct TasksForRanges {
+    tasks: Vec<Task<()>>,
+    sorted_ranges: Vec<Range<language::Anchor>>,
 }
 
 #[derive(Debug)]
@@ -32,7 +39,7 @@ pub struct CachedExcerptHints {
     version: usize,
     buffer_version: Global,
     buffer_id: u64,
-    pub hints: Vec<(InlayId, InlayHint)>,
+    hints: Vec<(InlayId, InlayHint)>,
 }
 
 #[derive(Debug, Clone, Copy)]
@@ -48,18 +55,6 @@ pub struct InlaySplice {
     pub to_insert: Vec<Inlay>,
 }
 
-struct UpdateTask {
-    invalidate: InvalidationStrategy,
-    cache_version: usize,
-    task: RunningTask,
-    pending_refresh: Option<ExcerptQuery>,
-}
-
-struct RunningTask {
-    _task: Task<()>,
-    is_running_rx: smol::channel::Receiver<()>,
-}
-
 #[derive(Debug)]
 struct ExcerptHintsUpdate {
     excerpt_id: ExcerptId,
@@ -72,24 +67,10 @@ struct ExcerptHintsUpdate {
 struct ExcerptQuery {
     buffer_id: u64,
     excerpt_id: ExcerptId,
-    dimensions: ExcerptDimensions,
     cache_version: usize,
     invalidate: InvalidationStrategy,
 }
 
-#[derive(Debug, Clone, Copy)]
-struct ExcerptDimensions {
-    excerpt_range_start: language::Anchor,
-    excerpt_range_end: language::Anchor,
-    excerpt_visible_range_start: language::Anchor,
-    excerpt_visible_range_end: language::Anchor,
-}
-
-struct HintFetchRanges {
-    visible_range: Range<language::Anchor>,
-    other_ranges: Vec<Range<language::Anchor>>,
-}
-
 impl InvalidationStrategy {
     fn should_invalidate(&self) -> bool {
         matches!(
@@ -99,35 +80,92 @@ impl InvalidationStrategy {
     }
 }
 
-impl ExcerptQuery {
-    fn hints_fetch_ranges(&self, buffer: &BufferSnapshot) -> HintFetchRanges {
-        let visible_range =
-            self.dimensions.excerpt_visible_range_start..self.dimensions.excerpt_visible_range_end;
-        let mut other_ranges = Vec::new();
-        if self
-            .dimensions
-            .excerpt_range_start
-            .cmp(&visible_range.start, buffer)
-            .is_lt()
-        {
-            let mut end = visible_range.start;
-            end.offset -= 1;
-            other_ranges.push(self.dimensions.excerpt_range_start..end);
-        }
-        if self
-            .dimensions
-            .excerpt_range_end
-            .cmp(&visible_range.end, buffer)
-            .is_gt()
-        {
-            let mut start = visible_range.end;
-            start.offset += 1;
-            other_ranges.push(start..self.dimensions.excerpt_range_end);
+impl TasksForRanges {
+    fn new(sorted_ranges: Vec<Range<language::Anchor>>, task: Task<()>) -> Self {
+        Self {
+            tasks: vec![task],
+            sorted_ranges,
         }
+    }
 
-        HintFetchRanges {
-            visible_range,
-            other_ranges: other_ranges.into_iter().map(|range| range).collect(),
+    fn update_cached_tasks(
+        &mut self,
+        buffer_snapshot: &BufferSnapshot,
+        query_range: Range<text::Anchor>,
+        invalidate: InvalidationStrategy,
+        spawn_task: impl FnOnce(Vec<Range<language::Anchor>>) -> Task<()>,
+    ) {
+        let ranges_to_query = match invalidate {
+            InvalidationStrategy::None => {
+                let mut ranges_to_query = Vec::new();
+                let mut latest_cached_range = None::<&mut Range<language::Anchor>>;
+                for cached_range in self
+                    .sorted_ranges
+                    .iter_mut()
+                    .skip_while(|cached_range| {
+                        cached_range
+                            .end
+                            .cmp(&query_range.start, buffer_snapshot)
+                            .is_lt()
+                    })
+                    .take_while(|cached_range| {
+                        cached_range
+                            .start
+                            .cmp(&query_range.end, buffer_snapshot)
+                            .is_le()
+                    })
+                {
+                    match latest_cached_range {
+                        Some(latest_cached_range) => {
+                            if latest_cached_range.end.offset.saturating_add(1)
+                                < cached_range.start.offset
+                            {
+                                ranges_to_query.push(latest_cached_range.end..cached_range.start);
+                                cached_range.start = latest_cached_range.end;
+                            }
+                        }
+                        None => {
+                            if query_range
+                                .start
+                                .cmp(&cached_range.start, buffer_snapshot)
+                                .is_lt()
+                            {
+                                ranges_to_query.push(query_range.start..cached_range.start);
+                                cached_range.start = query_range.start;
+                            }
+                        }
+                    }
+                    latest_cached_range = Some(cached_range);
+                }
+
+                match latest_cached_range {
+                    Some(latest_cached_range) => {
+                        if latest_cached_range.end.offset.saturating_add(1) < query_range.end.offset
+                        {
+                            ranges_to_query.push(latest_cached_range.end..query_range.end);
+                            latest_cached_range.end = query_range.end;
+                        }
+                    }
+                    None => {
+                        ranges_to_query.push(query_range.clone());
+                        self.sorted_ranges.push(query_range);
+                        self.sorted_ranges.sort_by(|range_a, range_b| {
+                            range_a.start.cmp(&range_b.start, buffer_snapshot)
+                        });
+                    }
+                }
+
+                ranges_to_query
+            }
+            InvalidationStrategy::RefreshRequested | InvalidationStrategy::BufferEdited => {
+                self.tasks.clear();
+                self.sorted_ranges.clear();
+                vec![query_range]
+            }
+        };
+
+        if !ranges_to_query.is_empty() {
+            self.tasks.push(spawn_task(ranges_to_query));
         }
     }
 }
@@ -168,7 +206,6 @@ impl InlayHintCache {
                     );
                     if new_splice.is_some() {
                         self.version += 1;
-                        self.update_tasks.clear();
                         self.allowed_hint_kinds = new_allowed_hint_kinds;
                     }
                     ControlFlow::Break(new_splice)
@@ -197,7 +234,7 @@ impl InlayHintCache {
 
     pub fn spawn_hint_refresh(
         &mut self,
-        mut excerpts_to_query: HashMap<ExcerptId, (ModelHandle<Buffer>, Global, Range<usize>)>,
+        excerpts_to_query: HashMap<ExcerptId, (ModelHandle<Buffer>, Global, Range<usize>)>,
         invalidate: InvalidationStrategy,
         cx: &mut ViewContext<Editor>,
     ) -> Option<InlaySplice> {
@@ -205,43 +242,23 @@ impl InlayHintCache {
             return None;
         }
 
-        let update_tasks = &mut self.update_tasks;
         let mut invalidated_hints = Vec::new();
         if invalidate.should_invalidate() {
-            let mut changed = false;
-            update_tasks.retain(|task_excerpt_id, _| {
-                let retain = excerpts_to_query.contains_key(task_excerpt_id);
-                changed |= !retain;
-                retain
-            });
+            self.update_tasks
+                .retain(|task_excerpt_id, _| excerpts_to_query.contains_key(task_excerpt_id));
             self.hints.retain(|cached_excerpt, cached_hints| {
                 let retain = excerpts_to_query.contains_key(cached_excerpt);
-                changed |= !retain;
                 if !retain {
                     invalidated_hints.extend(cached_hints.read().hints.iter().map(|&(id, _)| id));
                 }
                 retain
             });
-            if changed {
-                self.version += 1;
-            }
         }
         if excerpts_to_query.is_empty() && invalidated_hints.is_empty() {
             return None;
         }
 
-        let cache_version = self.version;
-        excerpts_to_query.retain(|visible_excerpt_id, _| {
-            match update_tasks.entry(*visible_excerpt_id) {
-                hash_map::Entry::Occupied(o) => match o.get().cache_version.cmp(&cache_version) {
-                    cmp::Ordering::Less => true,
-                    cmp::Ordering::Equal => invalidate.should_invalidate(),
-                    cmp::Ordering::Greater => false,
-                },
-                hash_map::Entry::Vacant(_) => true,
-            }
-        });
-
+        let cache_version = self.version + 1;
         cx.spawn(|editor, mut cx| async move {
             editor
                 .update(&mut cx, |editor, cx| {
@@ -368,6 +385,19 @@ impl InlayHintCache {
         self.update_tasks.clear();
         self.hints.clear();
     }
+
+    pub fn hints(&self) -> Vec<InlayHint> {
+        let mut hints = Vec::new();
+        for excerpt_hints in self.hints.values() {
+            let excerpt_hints = excerpt_hints.read();
+            hints.extend(excerpt_hints.hints.iter().map(|(_, hint)| hint).cloned());
+        }
+        hints
+    }
+
+    pub fn version(&self) -> usize {
+        self.version
+    }
 }
 
 fn spawn_new_update_tasks(
@@ -378,13 +408,14 @@ fn spawn_new_update_tasks(
     cx: &mut ViewContext<'_, '_, Editor>,
 ) {
     let visible_hints = Arc::new(editor.visible_inlay_hints(cx));
-    for (excerpt_id, (buffer_handle, new_task_buffer_version, excerpt_visible_range)) in
+    for (excerpt_id, (excerpt_buffer, new_task_buffer_version, excerpt_visible_range)) in
         excerpts_to_query
     {
         if excerpt_visible_range.is_empty() {
             continue;
         }
-        let buffer = buffer_handle.read(cx);
+        let buffer = excerpt_buffer.read(cx);
+        let buffer_id = buffer.remote_id();
         let buffer_snapshot = buffer.snapshot();
         if buffer_snapshot
             .version()
@@ -402,202 +433,123 @@ fn spawn_new_update_tasks(
             {
                 continue;
             }
-            if !new_task_buffer_version.changed_since(&cached_buffer_version)
-                && !matches!(invalidate, InvalidationStrategy::RefreshRequested)
-            {
-                continue;
-            }
         };
 
-        let buffer_id = buffer.remote_id();
-        let excerpt_visible_range_start = buffer.anchor_before(excerpt_visible_range.start);
-        let excerpt_visible_range_end = buffer.anchor_after(excerpt_visible_range.end);
-
-        let (multi_buffer_snapshot, full_excerpt_range) =
+        let (multi_buffer_snapshot, Some(query_range)) =
             editor.buffer.update(cx, |multi_buffer, cx| {
-                let multi_buffer_snapshot = multi_buffer.snapshot(cx);
                 (
-                    multi_buffer_snapshot,
-                    multi_buffer
-                        .excerpts_for_buffer(&buffer_handle, cx)
-                        .into_iter()
-                        .find(|(id, _)| id == &excerpt_id)
-                        .map(|(_, range)| range.context),
+                    multi_buffer.snapshot(cx),
+                    determine_query_range(
+                        multi_buffer,
+                        excerpt_id,
+                        &excerpt_buffer,
+                        excerpt_visible_range,
+                        cx,
+                    ),
                 )
-            });
+            }) else { return; };
+        let query = ExcerptQuery {
+            buffer_id,
+            excerpt_id,
+            cache_version: update_cache_version,
+            invalidate,
+        };
 
-        if let Some(full_excerpt_range) = full_excerpt_range {
-            let query = ExcerptQuery {
-                buffer_id,
-                excerpt_id,
-                dimensions: ExcerptDimensions {
-                    excerpt_range_start: full_excerpt_range.start,
-                    excerpt_range_end: full_excerpt_range.end,
-                    excerpt_visible_range_start,
-                    excerpt_visible_range_end,
-                },
-                cache_version: update_cache_version,
-                invalidate,
-            };
+        let new_update_task = |fetch_ranges| {
+            new_update_task(
+                query,
+                fetch_ranges,
+                multi_buffer_snapshot,
+                buffer_snapshot.clone(),
+                Arc::clone(&visible_hints),
+                cached_excerpt_hints,
+                cx,
+            )
+        };
 
-            let new_update_task = |is_refresh_after_regular_task| {
-                new_update_task(
-                    query,
-                    multi_buffer_snapshot,
-                    buffer_snapshot,
-                    Arc::clone(&visible_hints),
-                    cached_excerpt_hints,
-                    is_refresh_after_regular_task,
-                    cx,
-                )
-            };
-            match editor.inlay_hint_cache.update_tasks.entry(excerpt_id) {
-                hash_map::Entry::Occupied(mut o) => {
-                    let update_task = o.get_mut();
-                    match (update_task.invalidate, invalidate) {
-                        (_, InvalidationStrategy::None) => {}
-                        (
-                            InvalidationStrategy::BufferEdited,
-                            InvalidationStrategy::RefreshRequested,
-                        ) if !update_task.task.is_running_rx.is_closed() => {
-                            update_task.pending_refresh = Some(query);
-                        }
-                        _ => {
-                            o.insert(UpdateTask {
-                                invalidate,
-                                cache_version: query.cache_version,
-                                task: new_update_task(false),
-                                pending_refresh: None,
-                            });
-                        }
-                    }
-                }
-                hash_map::Entry::Vacant(v) => {
-                    v.insert(UpdateTask {
-                        invalidate,
-                        cache_version: query.cache_version,
-                        task: new_update_task(false),
-                        pending_refresh: None,
-                    });
-                }
+        match editor.inlay_hint_cache.update_tasks.entry(excerpt_id) {
+            hash_map::Entry::Occupied(mut o) => {
+                o.get_mut().update_cached_tasks(
+                    &buffer_snapshot,
+                    query_range,
+                    invalidate,
+                    new_update_task,
+                );
+            }
+            hash_map::Entry::Vacant(v) => {
+                v.insert(TasksForRanges::new(
+                    vec![query_range.clone()],
+                    new_update_task(vec![query_range]),
+                ));
             }
         }
     }
 }
 
+fn determine_query_range(
+    multi_buffer: &mut MultiBuffer,
+    excerpt_id: ExcerptId,
+    excerpt_buffer: &ModelHandle<Buffer>,
+    excerpt_visible_range: Range<usize>,
+    cx: &mut ModelContext<'_, MultiBuffer>,
+) -> Option<Range<language::Anchor>> {
+    let full_excerpt_range = multi_buffer
+        .excerpts_for_buffer(excerpt_buffer, cx)
+        .into_iter()
+        .find(|(id, _)| id == &excerpt_id)
+        .map(|(_, range)| range.context)?;
+
+    let buffer = excerpt_buffer.read(cx);
+    let excerpt_visible_len = excerpt_visible_range.end - excerpt_visible_range.start;
+    let start_offset = excerpt_visible_range
+        .start
+        .saturating_sub(excerpt_visible_len)
+        .max(full_excerpt_range.start.offset);
+    let start = buffer.anchor_before(buffer.clip_offset(start_offset, Bias::Left));
+    let end_offset = excerpt_visible_range
+        .end
+        .saturating_add(excerpt_visible_len)
+        .min(full_excerpt_range.end.offset)
+        .min(buffer.len());
+    let end = buffer.anchor_after(buffer.clip_offset(end_offset, Bias::Right));
+    if start.cmp(&end, buffer).is_eq() {
+        None
+    } else {
+        Some(start..end)
+    }
+}
+
 fn new_update_task(
     query: ExcerptQuery,
+    hint_fetch_ranges: Vec<Range<language::Anchor>>,
     multi_buffer_snapshot: MultiBufferSnapshot,
     buffer_snapshot: BufferSnapshot,
     visible_hints: Arc<Vec<Inlay>>,
     cached_excerpt_hints: Option<Arc<RwLock<CachedExcerptHints>>>,
-    is_refresh_after_regular_task: bool,
     cx: &mut ViewContext<'_, '_, Editor>,
-) -> RunningTask {
-    let hints_fetch_ranges = query.hints_fetch_ranges(&buffer_snapshot);
-    let (is_running_tx, is_running_rx) = smol::channel::bounded(1);
-    let _task = cx.spawn(|editor, mut cx| async move {
-        let _is_running_tx = is_running_tx;
-        let create_update_task = |range| {
-            fetch_and_update_hints(
-                editor.clone(),
-                multi_buffer_snapshot.clone(),
-                buffer_snapshot.clone(),
-                Arc::clone(&visible_hints),
-                cached_excerpt_hints.as_ref().map(Arc::clone),
-                query,
-                range,
-                cx.clone(),
-            )
-        };
-
-        if is_refresh_after_regular_task {
-            let visible_range_has_updates =
-                match create_update_task(hints_fetch_ranges.visible_range).await {
-                    Ok(updated) => updated,
-                    Err(e) => {
-                        error!("inlay hint visible range update task failed: {e:#}");
-                        return;
-                    }
-                };
-
-            if visible_range_has_updates {
-                let other_update_results = futures::future::join_all(
-                    hints_fetch_ranges
-                        .other_ranges
-                        .into_iter()
-                        .map(create_update_task),
+) -> Task<()> {
+    cx.spawn(|editor, cx| async move {
+        let task_update_results =
+            futures::future::join_all(hint_fetch_ranges.into_iter().map(|range| {
+                fetch_and_update_hints(
+                    editor.clone(),
+                    multi_buffer_snapshot.clone(),
+                    buffer_snapshot.clone(),
+                    Arc::clone(&visible_hints),
+                    cached_excerpt_hints.as_ref().map(Arc::clone),
+                    query,
+                    range,
+                    cx.clone(),
                 )
-                .await;
-
-                for result in other_update_results {
-                    if let Err(e) = result {
-                        error!("inlay hint update task failed: {e:#}");
-                    }
-                }
-            }
-        } else {
-            let task_update_results = futures::future::join_all(
-                std::iter::once(hints_fetch_ranges.visible_range)
-                    .chain(hints_fetch_ranges.other_ranges.into_iter())
-                    .map(create_update_task),
-            )
+            }))
             .await;
 
-            for result in task_update_results {
-                if let Err(e) = result {
-                    error!("inlay hint update task failed: {e:#}");
-                }
+        for result in task_update_results {
+            if let Err(e) = result {
+                error!("inlay hint update task failed: {e:#}");
             }
         }
-
-        editor
-            .update(&mut cx, |editor, cx| {
-                let pending_refresh_query = editor
-                    .inlay_hint_cache
-                    .update_tasks
-                    .get_mut(&query.excerpt_id)
-                    .and_then(|task| task.pending_refresh.take());
-
-                if let Some(pending_refresh_query) = pending_refresh_query {
-                    let refresh_multi_buffer = editor.buffer().read(cx);
-                    let refresh_multi_buffer_snapshot = refresh_multi_buffer.snapshot(cx);
-                    let refresh_visible_hints = Arc::new(editor.visible_inlay_hints(cx));
-                    let refresh_cached_excerpt_hints = editor
-                        .inlay_hint_cache
-                        .hints
-                        .get(&pending_refresh_query.excerpt_id)
-                        .map(Arc::clone);
-                    if let Some(buffer) =
-                        refresh_multi_buffer.buffer(pending_refresh_query.buffer_id)
-                    {
-                        editor.inlay_hint_cache.update_tasks.insert(
-                            pending_refresh_query.excerpt_id,
-                            UpdateTask {
-                                invalidate: InvalidationStrategy::RefreshRequested,
-                                cache_version: editor.inlay_hint_cache.version,
-                                task: new_update_task(
-                                    pending_refresh_query,
-                                    refresh_multi_buffer_snapshot,
-                                    buffer.read(cx).snapshot(),
-                                    refresh_visible_hints,
-                                    refresh_cached_excerpt_hints,
-                                    true,
-                                    cx,
-                                ),
-                                pending_refresh: None,
-                            },
-                        );
-                    }
-                }
-            })
-            .ok();
-    });
-
-    RunningTask {
-        _task,
-        is_running_rx,
-    }
+    })
 }
 
 async fn fetch_and_update_hints(
@@ -609,7 +561,7 @@ async fn fetch_and_update_hints(
     query: ExcerptQuery,
     fetch_range: Range<language::Anchor>,
     mut cx: gpui::AsyncAppContext,
-) -> anyhow::Result<bool> {
+) -> anyhow::Result<()> {
     let inlay_hints_fetch_task = editor
         .update(&mut cx, |editor, cx| {
             editor
@@ -625,11 +577,10 @@ async fn fetch_and_update_hints(
         })
         .ok()
         .flatten();
-    let mut update_happened = false;
-    let Some(inlay_hints_fetch_task) = inlay_hints_fetch_task else { return Ok(update_happened) };
-    let new_hints = inlay_hints_fetch_task
-        .await
-        .context("inlay hint fetch task")?;
+    let new_hints = match inlay_hints_fetch_task {
+        Some(task) => task.await.context("inlay hint fetch task")?,
+        None => return Ok(()),
+    };
     let background_task_buffer_snapshot = buffer_snapshot.clone();
     let backround_fetch_range = fetch_range.clone();
     let new_update = cx
@@ -645,106 +596,21 @@ async fn fetch_and_update_hints(
             )
         })
         .await;
-
-    editor
-        .update(&mut cx, |editor, cx| {
-            if let Some(new_update) = new_update {
-                update_happened = !new_update.add_to_cache.is_empty()
-                    || !new_update.remove_from_cache.is_empty()
-                    || !new_update.remove_from_visible.is_empty();
-
-                let cached_excerpt_hints = editor
-                    .inlay_hint_cache
-                    .hints
-                    .entry(new_update.excerpt_id)
-                    .or_insert_with(|| {
-                        Arc::new(RwLock::new(CachedExcerptHints {
-                            version: query.cache_version,
-                            buffer_version: buffer_snapshot.version().clone(),
-                            buffer_id: query.buffer_id,
-                            hints: Vec::new(),
-                        }))
-                    });
-                let mut cached_excerpt_hints = cached_excerpt_hints.write();
-                match query.cache_version.cmp(&cached_excerpt_hints.version) {
-                    cmp::Ordering::Less => return,
-                    cmp::Ordering::Greater | cmp::Ordering::Equal => {
-                        cached_excerpt_hints.version = query.cache_version;
-                    }
-                }
-                cached_excerpt_hints
-                    .hints
-                    .retain(|(hint_id, _)| !new_update.remove_from_cache.contains(hint_id));
-                cached_excerpt_hints.buffer_version = buffer_snapshot.version().clone();
-                editor.inlay_hint_cache.version += 1;
-
-                let mut splice = InlaySplice {
-                    to_remove: new_update.remove_from_visible,
-                    to_insert: Vec::new(),
-                };
-
-                for new_hint in new_update.add_to_cache {
-                    let new_hint_position = multi_buffer_snapshot
-                        .anchor_in_excerpt(query.excerpt_id, new_hint.position);
-                    let new_inlay_id = post_inc(&mut editor.next_inlay_id);
-                    if editor
-                        .inlay_hint_cache
-                        .allowed_hint_kinds
-                        .contains(&new_hint.kind)
-                    {
-                        splice.to_insert.push(Inlay::hint(
-                            new_inlay_id,
-                            new_hint_position,
-                            &new_hint,
-                        ));
-                    }
-
-                    cached_excerpt_hints
-                        .hints
-                        .push((InlayId::Hint(new_inlay_id), new_hint));
-                }
-
-                cached_excerpt_hints
-                    .hints
-                    .sort_by(|(_, hint_a), (_, hint_b)| {
-                        hint_a.position.cmp(&hint_b.position, &buffer_snapshot)
-                    });
-                drop(cached_excerpt_hints);
-
-                if query.invalidate.should_invalidate() {
-                    let mut outdated_excerpt_caches = HashSet::default();
-                    for (excerpt_id, excerpt_hints) in &editor.inlay_hint_cache().hints {
-                        let excerpt_hints = excerpt_hints.read();
-                        if excerpt_hints.buffer_id == query.buffer_id
-                            && excerpt_id != &query.excerpt_id
-                            && buffer_snapshot
-                                .version()
-                                .changed_since(&excerpt_hints.buffer_version)
-                        {
-                            outdated_excerpt_caches.insert(*excerpt_id);
-                            splice
-                                .to_remove
-                                .extend(excerpt_hints.hints.iter().map(|(id, _)| id));
-                        }
-                    }
-                    editor
-                        .inlay_hint_cache
-                        .hints
-                        .retain(|excerpt_id, _| !outdated_excerpt_caches.contains(excerpt_id));
-                }
-
-                let InlaySplice {
-                    to_remove,
-                    to_insert,
-                } = splice;
-                if !to_remove.is_empty() || !to_insert.is_empty() {
-                    editor.splice_inlay_hints(to_remove, to_insert, cx)
-                }
-            }
-        })
-        .ok();
-
-    Ok(update_happened)
+    if let Some(new_update) = new_update {
+        editor
+            .update(&mut cx, |editor, cx| {
+                apply_hint_update(
+                    editor,
+                    new_update,
+                    query,
+                    buffer_snapshot,
+                    multi_buffer_snapshot,
+                    cx,
+                );
+            })
+            .ok();
+    }
+    Ok(())
 }
 
 fn calculate_hint_updates(
@@ -793,19 +659,6 @@ fn calculate_hint_updates(
             visible_hints
                 .iter()
                 .filter(|hint| hint.position.excerpt_id == query.excerpt_id)
-                .filter(|hint| {
-                    contains_position(&fetch_range, hint.position.text_anchor, buffer_snapshot)
-                })
-                .filter(|hint| {
-                    fetch_range
-                        .start
-                        .cmp(&hint.position.text_anchor, buffer_snapshot)
-                        .is_le()
-                        && fetch_range
-                            .end
-                            .cmp(&hint.position.text_anchor, buffer_snapshot)
-                            .is_ge()
-                })
                 .map(|inlay_hint| inlay_hint.id)
                 .filter(|hint_id| !excerpt_hints_to_persist.contains_key(hint_id)),
         );
@@ -819,16 +672,6 @@ fn calculate_hint_updates(
                     .filter(|(cached_inlay_id, _)| {
                         !excerpt_hints_to_persist.contains_key(cached_inlay_id)
                     })
-                    .filter(|(_, cached_hint)| {
-                        fetch_range
-                            .start
-                            .cmp(&cached_hint.position, buffer_snapshot)
-                            .is_le()
-                            && fetch_range
-                                .end
-                                .cmp(&cached_hint.position, buffer_snapshot)
-                                .is_ge()
-                    })
                     .map(|(cached_inlay_id, _)| *cached_inlay_id),
             );
         }
@@ -855,6 +698,113 @@ fn contains_position(
         && range.end.cmp(&position, buffer_snapshot).is_ge()
 }
 
+fn apply_hint_update(
+    editor: &mut Editor,
+    new_update: ExcerptHintsUpdate,
+    query: ExcerptQuery,
+    buffer_snapshot: BufferSnapshot,
+    multi_buffer_snapshot: MultiBufferSnapshot,
+    cx: &mut ViewContext<'_, '_, Editor>,
+) {
+    let cached_excerpt_hints = editor
+        .inlay_hint_cache
+        .hints
+        .entry(new_update.excerpt_id)
+        .or_insert_with(|| {
+            Arc::new(RwLock::new(CachedExcerptHints {
+                version: query.cache_version,
+                buffer_version: buffer_snapshot.version().clone(),
+                buffer_id: query.buffer_id,
+                hints: Vec::new(),
+            }))
+        });
+    let mut cached_excerpt_hints = cached_excerpt_hints.write();
+    match query.cache_version.cmp(&cached_excerpt_hints.version) {
+        cmp::Ordering::Less => return,
+        cmp::Ordering::Greater | cmp::Ordering::Equal => {
+            cached_excerpt_hints.version = query.cache_version;
+        }
+    }
+
+    let mut cached_inlays_changed = !new_update.remove_from_cache.is_empty();
+    cached_excerpt_hints
+        .hints
+        .retain(|(hint_id, _)| !new_update.remove_from_cache.contains(hint_id));
+    let mut splice = InlaySplice {
+        to_remove: new_update.remove_from_visible,
+        to_insert: Vec::new(),
+    };
+    for new_hint in new_update.add_to_cache {
+        let cached_hints = &mut cached_excerpt_hints.hints;
+        let insert_position = match cached_hints
+            .binary_search_by(|probe| probe.1.position.cmp(&new_hint.position, &buffer_snapshot))
+        {
+            Ok(i) => {
+                if cached_hints[i].1.text() == new_hint.text() {
+                    None
+                } else {
+                    Some(i)
+                }
+            }
+            Err(i) => Some(i),
+        };
+
+        if let Some(insert_position) = insert_position {
+            let new_inlay_id = post_inc(&mut editor.next_inlay_id);
+            if editor
+                .inlay_hint_cache
+                .allowed_hint_kinds
+                .contains(&new_hint.kind)
+            {
+                let new_hint_position =
+                    multi_buffer_snapshot.anchor_in_excerpt(query.excerpt_id, new_hint.position);
+                splice
+                    .to_insert
+                    .push(Inlay::hint(new_inlay_id, new_hint_position, &new_hint));
+            }
+            cached_hints.insert(insert_position, (InlayId::Hint(new_inlay_id), new_hint));
+            cached_inlays_changed = true;
+        }
+    }
+    cached_excerpt_hints.buffer_version = buffer_snapshot.version().clone();
+    drop(cached_excerpt_hints);
+
+    if query.invalidate.should_invalidate() {
+        let mut outdated_excerpt_caches = HashSet::default();
+        for (excerpt_id, excerpt_hints) in &editor.inlay_hint_cache().hints {
+            let excerpt_hints = excerpt_hints.read();
+            if excerpt_hints.buffer_id == query.buffer_id
+                && excerpt_id != &query.excerpt_id
+                && buffer_snapshot
+                    .version()
+                    .changed_since(&excerpt_hints.buffer_version)
+            {
+                outdated_excerpt_caches.insert(*excerpt_id);
+                splice
+                    .to_remove
+                    .extend(excerpt_hints.hints.iter().map(|(id, _)| id));
+            }
+        }
+        cached_inlays_changed |= !outdated_excerpt_caches.is_empty();
+        editor
+            .inlay_hint_cache
+            .hints
+            .retain(|excerpt_id, _| !outdated_excerpt_caches.contains(excerpt_id));
+    }
+
+    let InlaySplice {
+        to_remove,
+        to_insert,
+    } = splice;
+    let displayed_inlays_changed = !to_remove.is_empty() || !to_insert.is_empty();
+    if cached_inlays_changed || displayed_inlays_changed {
+        editor.inlay_hint_cache.version += 1;
+    }
+    if displayed_inlays_changed {
+        editor.splice_inlay_hints(to_remove, to_insert, cx)
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use std::sync::atomic::{AtomicBool, AtomicU32, Ordering};
@@ -866,6 +816,7 @@ mod tests {
     };
     use futures::StreamExt;
     use gpui::{executor::Deterministic, TestAppContext, ViewHandle};
+    use itertools::Itertools;
     use language::{
         language_settings::AllLanguageSettingsContent, FakeLspAdapter, Language, LanguageConfig,
     };
@@ -873,7 +824,7 @@ mod tests {
     use parking_lot::Mutex;
     use project::{FakeFs, Project};
     use settings::SettingsStore;
-    use text::Point;
+    use text::{Point, ToPoint};
     use workspace::Workspace;
 
     use crate::editor_tests::update_test_language_settings;
@@ -1879,7 +1830,7 @@ mod tests {
 
                     task_lsp_request_ranges.lock().push(params.range);
                     let query_start = params.range.start;
-                    let i = Arc::clone(&task_lsp_request_count).fetch_add(1, Ordering::SeqCst) + 1;
+                    let i = Arc::clone(&task_lsp_request_count).fetch_add(1, Ordering::Release) + 1;
                     Ok(Some(vec![lsp::InlayHint {
                         position: query_start,
                         label: lsp::InlayHintLabel::String(i.to_string()),
@@ -1894,18 +1845,44 @@ mod tests {
             })
             .next()
             .await;
+        fn editor_visible_range(
+            editor: &ViewHandle<Editor>,
+            cx: &mut gpui::TestAppContext,
+        ) -> Range<Point> {
+            let ranges = editor.update(cx, |editor, cx| editor.excerpt_visible_offsets(None, cx));
+            assert_eq!(
+                ranges.len(),
+                1,
+                "Single buffer should produce a single excerpt with visible range"
+            );
+            let (_, (excerpt_buffer, _, excerpt_visible_range)) =
+                ranges.into_iter().next().unwrap();
+            excerpt_buffer.update(cx, |buffer, _| {
+                let snapshot = buffer.snapshot();
+                let start = buffer
+                    .anchor_before(excerpt_visible_range.start)
+                    .to_point(&snapshot);
+                let end = buffer
+                    .anchor_after(excerpt_visible_range.end)
+                    .to_point(&snapshot);
+                start..end
+            })
+        }
+
+        let initial_visible_range = editor_visible_range(&editor, cx);
+        let expected_initial_query_range_end =
+            lsp::Position::new(initial_visible_range.end.row * 2, 1);
         cx.foreground().run_until_parked();
         editor.update(cx, |editor, cx| {
-            let mut ranges = lsp_request_ranges.lock().drain(..).collect::<Vec<_>>();
-            ranges.sort_by_key(|range| range.start);
-            assert_eq!(ranges.len(), 2, "When scroll is at the edge of a big document, its visible part + the rest should be queried for hints");
-            assert_eq!(ranges[0].start, lsp::Position::new(0, 0), "Should query from the beginning of the document");
-            assert_eq!(ranges[0].end.line, ranges[1].start.line, "Both requests should be on the same line");
-            assert_eq!(ranges[0].end.character + 1, ranges[1].start.character, "Both request should be concequent");
-
-            assert_eq!(lsp_request_count.load(Ordering::SeqCst), 2,
-                "When scroll is at the edge of a big document, its visible part + the rest should be queried for hints");
-            let expected_layers = vec!["1".to_string(), "2".to_string()];
+            let ranges = lsp_request_ranges.lock().drain(..).collect::<Vec<_>>();
+            assert_eq!(ranges.len(), 1,
+                "When scroll is at the edge of a big document, double of its visible part range should be queried for hints in one single big request, but got: {ranges:?}");
+            let query_range = &ranges[0];
+            assert_eq!(query_range.start, lsp::Position::new(0, 0), "Should query initially from the beginning of the document");
+            assert_eq!(query_range.end, expected_initial_query_range_end, "Should query initially for double lines of the visible part of the document");
+
+            assert_eq!(lsp_request_count.load(Ordering::Acquire), 1);
+            let expected_layers = vec!["1".to_string()];
             assert_eq!(
                 expected_layers,
                 cached_hint_labels(editor),
@@ -1913,37 +1890,114 @@ mod tests {
             );
             assert_eq!(expected_layers, visible_hint_labels(editor, cx));
             assert_eq!(
-                editor.inlay_hint_cache().version, 2,
-                "Both LSP queries should've bumped the cache version"
+                editor.inlay_hint_cache().version, 1,
+                "LSP queries should've bumped the cache version"
             );
         });
 
         editor.update(cx, |editor, cx| {
             editor.scroll_screen(&ScrollAmount::Page(1.0), cx);
             editor.scroll_screen(&ScrollAmount::Page(1.0), cx);
-            editor.change_selections(None, cx, |s| s.select_ranges([600..600]));
-            editor.handle_input("++++more text++++", cx);
         });
 
+        let visible_range_after_scrolls = editor_visible_range(&editor, cx);
+        let visible_line_count =
+            editor.update(cx, |editor, _| editor.visible_line_count().unwrap());
         cx.foreground().run_until_parked();
+        let selection_in_cached_range = editor.update(cx, |editor, cx| {
+            let ranges = lsp_request_ranges
+                .lock()
+                .drain(..)
+                .sorted_by_key(|r| r.start)
+                .collect::<Vec<_>>();
+            assert_eq!(
+                ranges.len(),
+                2,
+                "Should query 2 ranges after both scrolls, but got: {ranges:?}"
+            );
+            let first_scroll = &ranges[0];
+            let second_scroll = &ranges[1];
+            assert_eq!(
+                first_scroll.end, second_scroll.start,
+                "Should query 2 adjacent ranges after the scrolls, but got: {ranges:?}"
+            );
+            assert_eq!(
+                first_scroll.start, expected_initial_query_range_end,
+                "First scroll should start the query right after the end of the original scroll",
+            );
+            assert_eq!(
+                second_scroll.end,
+                lsp::Position::new(
+                    visible_range_after_scrolls.end.row
+                        + visible_line_count.ceil() as u32,
+                    0
+                ),
+                "Second scroll should query one more screen down after the end of the visible range"
+            );
+
+            assert_eq!(
+                lsp_request_count.load(Ordering::Acquire),
+                3,
+                "Should query for hints after every scroll"
+            );
+            let expected_layers = vec!["1".to_string(), "2".to_string(), "3".to_string()];
+            assert_eq!(
+                expected_layers,
+                cached_hint_labels(editor),
+                "Should have hints from the new LSP response after the edit"
+            );
+            assert_eq!(expected_layers, visible_hint_labels(editor, cx));
+            assert_eq!(
+                editor.inlay_hint_cache().version,
+                3,
+                "Should update the cache for every LSP response with hints added"
+            );
+
+            let mut selection_in_cached_range = visible_range_after_scrolls.end;
+            selection_in_cached_range.row -= visible_line_count.ceil() as u32;
+            selection_in_cached_range
+        });
+
+        editor.update(cx, |editor, cx| {
+            editor.change_selections(Some(Autoscroll::center()), cx, |s| {
+                s.select_ranges([selection_in_cached_range..selection_in_cached_range])
+            });
+        });
+        cx.foreground().run_until_parked();
+        editor.update(cx, |_, _| {
+            let ranges = lsp_request_ranges
+                .lock()
+                .drain(..)
+                .sorted_by_key(|r| r.start)
+                .collect::<Vec<_>>();
+            assert!(ranges.is_empty(), "No new ranges or LSP queries should be made after returning to the selection with cached hints");
+            assert_eq!(lsp_request_count.load(Ordering::Acquire), 3);
+        });
+
         editor.update(cx, |editor, cx| {
-            let mut ranges = lsp_request_ranges.lock().drain(..).collect::<Vec<_>>();
-            ranges.sort_by_key(|range| range.start);
-            assert_eq!(ranges.len(), 3, "When scroll is at the middle of a big document, its visible part + 2 other inbisible parts should be queried for hints");
-            assert_eq!(ranges[0].start, lsp::Position::new(0, 0), "Should query from the beginning of the document");
-            assert_eq!(ranges[0].end.line + 1, ranges[1].start.line, "Neighbour requests got on different lines due to the line end");
-            assert_ne!(ranges[0].end.character, 0, "First query was in the end of the line, not in the beginning");
-            assert_eq!(ranges[1].start.character, 0, "Second query got pushed into a new line and starts from the beginning");
-            assert_eq!(ranges[1].end.line, ranges[2].start.line, "Neighbour requests should be on the same line");
-            assert_eq!(ranges[1].end.character + 1, ranges[2].start.character, "Neighbour request should be concequent");
-
-            assert_eq!(lsp_request_count.load(Ordering::SeqCst), 5,
-                "When scroll not at the edge of a big document, visible part + 2 other parts should be queried for hints");
-            let expected_layers = vec!["3".to_string(), "4".to_string(), "5".to_string()];
+            editor.handle_input("++++more text++++", cx);
+        });
+        cx.foreground().run_until_parked();
+        editor.update(cx, |editor, cx| {
+            let ranges = lsp_request_ranges.lock().drain(..).collect::<Vec<_>>();
+            assert_eq!(ranges.len(), 1,
+                "On edit, should scroll to selection and query a range around it. Instead, got query ranges {ranges:?}");
+            let query_range = &ranges[0];
+            assert!(query_range.start.line < selection_in_cached_range.row,
+                "Hints should be queried with the selected range after the query range start");
+            assert!(query_range.end.line > selection_in_cached_range.row,
+                "Hints should be queried with the selected range before the query range end");
+            assert!(query_range.start.line <= selection_in_cached_range.row - (visible_line_count * 3.0 / 2.0) as u32,
+                "Hints query range should contain one more screen before");
+            assert!(query_range.end.line >= selection_in_cached_range.row + (visible_line_count * 3.0 / 2.0) as u32,
+                "Hints query range should contain one more screen after");
+
+            assert_eq!(lsp_request_count.load(Ordering::Acquire), 4, "Should query for hints once after the edit");
+            let expected_layers = vec!["4".to_string()];
             assert_eq!(expected_layers, cached_hint_labels(editor),
-                "Should have hints from the new LSP response after edit");
+                "Should have hints from the new LSP response after the edit");
             assert_eq!(expected_layers, visible_hint_labels(editor, cx));
-            assert_eq!(editor.inlay_hint_cache().version, 5, "Should update the cache for every LSP response with hints added");
+            assert_eq!(editor.inlay_hint_cache().version, 4, "Should update the cache for every LSP response with hints added");
         });
     }
 

crates/editor/src/scroll.rs 🔗

@@ -13,7 +13,7 @@ use gpui::{
 };
 use language::{Bias, Point};
 use util::ResultExt;
-use workspace::{item::Item, WorkspaceId};
+use workspace::WorkspaceId;
 
 use crate::{
     display_map::{DisplaySnapshot, ToDisplayPoint},
@@ -333,9 +333,7 @@ impl Editor {
             cx,
         );
 
-        if !self.is_singleton(cx) {
-            self.refresh_inlays(InlayRefreshReason::NewLinesShown, cx);
-        }
+        self.refresh_inlays(InlayRefreshReason::NewLinesShown, cx);
     }
 
     pub fn scroll_position(&self, cx: &mut ViewContext<Self>) -> Vector2F {