From 4d4544f680f5bf78ca013c7616417082c7380acc Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Sat, 24 Jun 2023 02:37:03 +0300 Subject: [PATCH] Split excerpts into mutliple ranges for inlay hint queries --- crates/collab/src/tests/integration_tests.rs | 52 +- crates/editor/src/editor.rs | 6 +- crates/editor/src/inlay_hint_cache.rs | 503 ++++++++++++------- 3 files changed, 349 insertions(+), 212 deletions(-) diff --git a/crates/collab/src/tests/integration_tests.rs b/crates/collab/src/tests/integration_tests.rs index d6f9846ab5fc7205845fd7b2521144afd362075c..288bc8e7afaa5c972eba2b9cb6fbf09af6043249 100644 --- a/crates/collab/src/tests/integration_tests.rs +++ b/crates/collab/src/tests/integration_tests.rs @@ -7817,6 +7817,10 @@ async fn test_mutual_editor_inlay_hint_cache_update( .create_room(&mut [(&client_a, cx_a), (&client_b, cx_b)]) .await; let active_call_a = cx_a.read(ActiveCall::global); + let active_call_b = cx_b.read(ActiveCall::global); + + cx_a.update(editor::init); + cx_b.update(editor::init); cx_a.update(|cx| { cx.update_global(|store: &mut SettingsStore, cx| { @@ -7876,22 +7880,30 @@ async fn test_mutual_editor_inlay_hint_cache_update( ) .await; let (project_a, worktree_id) = client_a.build_local_project("/a", cx_a).await; + active_call_a + .update(cx_a, |call, cx| call.set_location(Some(&project_a), cx)) + .await + .unwrap(); let project_id = active_call_a .update(cx_a, |call, cx| call.share_project(project_a.clone(), cx)) .await .unwrap(); - let buffer_a = project_a - .update(cx_a, |p, cx| p.open_buffer((worktree_id, "main.rs"), cx)) + + let project_b = client_b.build_remote_project(project_id, cx_b).await; + active_call_b + .update(cx_b, |call, cx| call.set_location(Some(&project_b), cx)) .await .unwrap(); - let (window_a, _) = cx_a.add_window(|_| EmptyView); - let editor_a = cx_a.add_view(window_a, |cx| { - Editor::for_buffer(buffer_a, Some(project_a.clone()), cx) - }); - editor_a.update(cx_a, |editor, cx| { - editor.set_scroll_position(vec2f(0., 1.), cx); - cx.focus(&editor_a) - }); + + let workspace_a = client_a.build_workspace(&project_a, cx_a); + let editor_a = workspace_a + .update(cx_a, |workspace, cx| { + workspace.open_path((worktree_id, "main.rs"), None, true, cx) + }) + .await + .unwrap() + .downcast::() + .unwrap(); cx_a.foreground().run_until_parked(); editor_a.update(cx_a, |editor, _| { assert!( @@ -7908,20 +7920,16 @@ async fn test_mutual_editor_inlay_hint_cache_update( "New cache should have no version updates" ); }); - - let project_b = client_b.build_remote_project(project_id, cx_b).await; - let buffer_b = project_b - .update(cx_b, |p, cx| p.open_buffer((worktree_id, "main.rs"), cx)) + let workspace_b = client_b.build_workspace(&project_b, cx_b); + let editor_b = workspace_b + .update(cx_b, |workspace, cx| { + workspace.open_path((worktree_id, "main.rs"), None, true, cx) + }) .await + .unwrap() + .downcast::() .unwrap(); - let (window_b, _) = cx_b.add_window(|_| EmptyView); - let editor_b = cx_b.add_view(window_b, |cx| { - Editor::for_buffer(buffer_b, Some(project_b.clone()), cx) - }); - editor_b.update(cx_b, |editor, cx| { - editor.set_scroll_position(vec2f(0., 1.), cx); - cx.focus(&editor_b) - }); + cx_b.foreground().run_until_parked(); editor_b.update(cx_b, |editor, _| { assert!( diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index dea36559539cf529a217b65b8832149f66b83c27..b5b5d550ce15e785a4b15434b3c49ca3387ee56d 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -2643,8 +2643,10 @@ impl Editor { (excerpt_id, (buffer, excerpt_visible_range)) }) .collect::>(); - self.inlay_hint_cache - .spawn_hints_update(excerpts_to_query, invalidate_cache, cx) + if !excerpts_to_query.is_empty() { + self.inlay_hint_cache + .refresh_inlay_hints(excerpts_to_query, invalidate_cache, cx) + } } fn excerpt_visible_offsets( diff --git a/crates/editor/src/inlay_hint_cache.rs b/crates/editor/src/inlay_hint_cache.rs index d75ecbe8882931f579e6c789ec18e84a301f477e..0b5902938392ac7238ff8681fa7170a99cee4730 100644 --- a/crates/editor/src/inlay_hint_cache.rs +++ b/crates/editor/src/inlay_hint_cache.rs @@ -148,7 +148,7 @@ impl InlayHintCache { } } - pub fn spawn_hints_update( + pub fn refresh_inlay_hints( &mut self, mut excerpts_to_query: HashMap, Range)>, invalidate: InvalidationStrategy, @@ -193,74 +193,7 @@ impl InlayHintCache { cx.spawn(|editor, mut cx| async move { editor .update(&mut cx, |editor, cx| { - let visible_hints = - Arc::new(visible_inlay_hints(editor, cx).cloned().collect::>()); - for (excerpt_id, (buffer_handle, excerpt_visible_range)) in excerpts_to_query { - if !excerpt_visible_range.is_empty() { - let buffer = buffer_handle.read(cx); - let buffer_snapshot = buffer.snapshot(); - let cached_excxerpt_hints = - editor.inlay_hint_cache.hints.get(&excerpt_id).cloned(); - if let Some(cached_excerpt_hints) = &cached_excxerpt_hints { - let new_task_buffer_version = buffer_snapshot.version(); - let cached_excerpt_hints = cached_excerpt_hints.read(); - let cached_buffer_version = &cached_excerpt_hints.buffer_version; - if cached_buffer_version.changed_since(new_task_buffer_version) { - return; - } - if !new_task_buffer_version.changed_since(&cached_buffer_version) - && !matches!(invalidate, InvalidationStrategy::All) - { - return; - } - } - - let buffer_id = buffer.remote_id(); - let excerpt_range_start = - buffer.anchor_before(excerpt_visible_range.start); - let excerpt_range_end = buffer.anchor_after(excerpt_visible_range.end); - - let (multi_buffer_snapshot, full_excerpt_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), - ) - }); - - if let Some(full_excerpt_range) = full_excerpt_range { - let query = ExcerptQuery { - buffer_id, - excerpt_id, - dimensions: ExcerptDimensions { - excerpt_range_start, - excerpt_range_end, - excerpt_visible_range_start: full_excerpt_range.start, - excerpt_visible_range_end: full_excerpt_range.end, - }, - cache_version, - invalidate, - }; - - editor.inlay_hint_cache.update_tasks.insert( - excerpt_id, - new_update_task( - query, - multi_buffer_snapshot, - buffer_snapshot, - Arc::clone(&visible_hints), - cached_excxerpt_hints, - cx, - ), - ); - } - } - } + spawn_new_update_tasks(editor, excerpts_to_query, invalidate, cache_version, cx) }) .ok(); }) @@ -377,6 +310,81 @@ impl InlayHintCache { } } +fn spawn_new_update_tasks( + editor: &mut Editor, + excerpts_to_query: HashMap, Range)>, + invalidate: InvalidationStrategy, + cache_version: usize, + cx: &mut ViewContext<'_, '_, Editor>, +) { + let visible_hints = Arc::new(visible_inlay_hints(editor, cx).cloned().collect::>()); + for (excerpt_id, (buffer_handle, excerpt_visible_range)) in excerpts_to_query { + if !excerpt_visible_range.is_empty() { + let buffer = buffer_handle.read(cx); + let buffer_snapshot = buffer.snapshot(); + let cached_excerpt_hints = editor.inlay_hint_cache.hints.get(&excerpt_id).cloned(); + 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_buffer_version = &cached_excerpt_hints.buffer_version; + if cached_buffer_version.changed_since(new_task_buffer_version) { + return; + } + // TODO kb on refresh (InvalidationStrategy::All), instead spawn a new task afterwards, that would wait before 1st query finishes + if !new_task_buffer_version.changed_since(&cached_buffer_version) + && !matches!(invalidate, InvalidationStrategy::All) + { + return; + } + } + + 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) = + 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), + ) + }); + + 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, + invalidate, + }; + + editor.inlay_hint_cache.update_tasks.insert( + excerpt_id, + new_update_task( + query, + multi_buffer_snapshot, + buffer_snapshot, + Arc::clone(&visible_hints), + cached_excerpt_hints, + cx, + ), + ); + } + } + } +} + fn new_update_task( query: ExcerptQuery, multi_buffer_snapshot: MultiBufferSnapshot, @@ -385,107 +393,166 @@ fn new_update_task( cached_excerpt_hints: Option>>, cx: &mut ViewContext<'_, '_, Editor>, ) -> InlayHintUpdateTask { - let hints_fetch_task = hints_fetch_task(query, cx); + let hints_fetch_tasks = hints_fetch_tasks(query, &buffer_snapshot, cx); + let _task = cx.spawn(|editor, cx| async move { + let create_update_task = |range, hint_fetch_task| { + 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, + hint_fetch_task, + cx.clone(), + ) + }; + + let (visible_range, visible_range_hint_fetch_task) = hints_fetch_tasks.visible_range; + let visible_range_has_updates = + match create_update_task(visible_range, visible_range_hint_fetch_task).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_tasks.other_ranges.into_iter().map( + |(fetch_range, hints_fetch_task)| { + create_update_task(fetch_range, hints_fetch_task) + }, + )) + .await; + + for result in other_update_results { + if let Err(e) = result { + error!("inlay hint update task failed: {e:#}"); + return; + } + } + } + }); + InlayHintUpdateTask { version: query.cache_version, - _task: cx.spawn(|editor, mut cx| async move { - match hints_fetch_task.await { - Ok(Some(new_hints)) => { - let task_buffer_snapshot = buffer_snapshot.clone(); - if let Some(new_update) = cx - .background() - .spawn(async move { - new_excerpt_hints_update_result( - query, - new_hints, - &task_buffer_snapshot, - cached_excerpt_hints, - &visible_hints, - ) - }) - .await - { - editor - .update(&mut cx, |editor, cx| { - let cached_excerpt_hints = editor - .inlay_hint_cache - .hints - .entry(new_update.excerpt_id) - .or_insert_with(|| { - Arc::new(RwLock::new(CachedExcerptHints { - version: new_update.cache_version, - buffer_version: buffer_snapshot.version().clone(), - hints: Vec::new(), - })) - }); - let mut cached_excerpt_hints = cached_excerpt_hints.write(); - match new_update.cache_version.cmp(&cached_excerpt_hints.version) { - cmp::Ordering::Less => return, - cmp::Ordering::Greater | cmp::Ordering::Equal => { - cached_excerpt_hints.version = new_update.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 = InlayId(post_inc(&mut editor.next_inlay_id)); - if editor - .inlay_hint_cache - .allowed_hint_kinds - .contains(&new_hint.kind) - { - splice.to_insert.push(( - new_hint_position, - new_inlay_id, - new_hint.clone(), - )); - } + _task, + } +} - cached_excerpt_hints.hints.push((new_inlay_id, new_hint)); - } +async fn fetch_and_update_hints( + editor: gpui::WeakViewHandle, + multi_buffer_snapshot: MultiBufferSnapshot, + buffer_snapshot: BufferSnapshot, + visible_hints: Arc>, + cached_excerpt_hints: Option>>, + query: ExcerptQuery, + fetch_range: Range, + hints_fetch_task: Task>>>, + mut cx: gpui::AsyncAppContext, +) -> anyhow::Result { + let mut update_happened = false; + match hints_fetch_task.await.context("inlay hint fetch task")? { + Some(new_hints) => { + let background_task_buffer_snapshot = buffer_snapshot.clone(); + let backround_fetch_range = fetch_range.clone(); + if let Some(new_update) = cx + .background() + .spawn(async move { + calculate_hint_updates( + query, + backround_fetch_range, + new_hints, + &background_task_buffer_snapshot, + cached_excerpt_hints, + &visible_hints, + ) + }) + .await + { + update_happened = !new_update.add_to_cache.is_empty() + || !new_update.remove_from_cache.is_empty() + || !new_update.remove_from_visible.is_empty(); + editor + .update(&mut cx, |editor, cx| { + let cached_excerpt_hints = editor + .inlay_hint_cache + .hints + .entry(new_update.excerpt_id) + .or_insert_with(|| { + Arc::new(RwLock::new(CachedExcerptHints { + version: new_update.cache_version, + buffer_version: buffer_snapshot.version().clone(), + hints: Vec::new(), + })) + }); + let mut cached_excerpt_hints = cached_excerpt_hints.write(); + match new_update.cache_version.cmp(&cached_excerpt_hints.version) { + cmp::Ordering::Less => return, + cmp::Ordering::Greater | cmp::Ordering::Equal => { + cached_excerpt_hints.version = new_update.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 = InlayId(post_inc(&mut editor.next_inlay_id)); + if editor + .inlay_hint_cache + .allowed_hint_kinds + .contains(&new_hint.kind) + { + splice.to_insert.push(( + new_hint_position, + new_inlay_id, + new_hint.clone(), + )); + } - cached_excerpt_hints - .hints - .sort_by(|(_, hint_a), (_, hint_b)| { - hint_a.position.cmp(&hint_b.position, &buffer_snapshot) - }); - drop(cached_excerpt_hints); - - 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(None) => {} - Err(e) => error!( - "Failed to fecth hints for excerpt {:?} in buffer {} : {}", - query.excerpt_id, query.buffer_id, e - ), + cached_excerpt_hints.hints.push((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); + + 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(); } - }), + } + None => {} } + + Ok(update_happened) } -fn new_excerpt_hints_update_result( +fn calculate_hint_updates( query: ExcerptQuery, + fetch_range: Range, new_excerpt_hints: Vec, buffer_snapshot: &BufferSnapshot, cached_excerpt_hints: Option>>, @@ -543,6 +610,16 @@ fn new_excerpt_hints_update_result( false, ) }) + .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)), ); @@ -556,6 +633,16 @@ fn new_excerpt_hints_update_result( .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), ); } @@ -590,37 +677,77 @@ fn allowed_hint_types( new_allowed_hint_types } -fn hints_fetch_task( +struct HintFetchTasks { + visible_range: ( + Range, + Task>>>, + ), + other_ranges: Vec<( + Range, + Task>>>, + )>, +} + +fn hints_fetch_tasks( query: ExcerptQuery, + buffer: &BufferSnapshot, cx: &mut ViewContext<'_, '_, Editor>, -) -> Task>>> { - cx.spawn(|editor, mut cx| async move { - let task = editor - .update(&mut cx, |editor, cx| { - editor - .buffer() - .read(cx) - .buffer(query.buffer_id) - .and_then(|buffer| { - let project = editor.project.as_ref()?; - Some(project.update(cx, |project, cx| { - project.inlay_hints( - buffer, - // TODO kb split into 3 queries - query.dimensions.excerpt_range_start - ..query.dimensions.excerpt_range_end, - cx, - ) - })) - }) +) -> HintFetchTasks { + let visible_range = + query.dimensions.excerpt_visible_range_start..query.dimensions.excerpt_visible_range_end; + let mut other_ranges = Vec::new(); + if query + .dimensions + .excerpt_range_start + .cmp(&query.dimensions.excerpt_visible_range_start, buffer) + .is_lt() + { + let mut end = query.dimensions.excerpt_visible_range_start; + end.offset -= 1; + other_ranges.push(query.dimensions.excerpt_range_start..end); + } + if query + .dimensions + .excerpt_range_end + .cmp(&query.dimensions.excerpt_visible_range_end, buffer) + .is_gt() + { + let mut start = query.dimensions.excerpt_visible_range_end; + start.offset += 1; + other_ranges.push(start..query.dimensions.excerpt_range_end); + } + + let mut query_task_for_range = |range_to_query| { + cx.spawn(|editor, mut cx| async move { + let task = editor + .update(&mut cx, |editor, cx| { + editor + .buffer() + .read(cx) + .buffer(query.buffer_id) + .and_then(|buffer| { + let project = editor.project.as_ref()?; + Some(project.update(cx, |project, cx| { + project.inlay_hints(buffer, range_to_query, cx) + })) + }) + }) + .ok() + .flatten(); + anyhow::Ok(match task { + Some(task) => Some(task.await.context("inlays for buffer task")?), + None => None, }) - .ok() - .flatten(); - Ok(match task { - Some(task) => Some(task.await.context("inlays for buffer task")?), - None => None, }) - }) + }; + + HintFetchTasks { + visible_range: (visible_range.clone(), query_task_for_range(visible_range)), + other_ranges: other_ranges + .into_iter() + .map(|range| (range.clone(), query_task_for_range(range))) + .collect(), + } } pub fn visible_inlay_hints<'a, 'b: 'a, 'c, 'd: 'a>(