diff --git a/crates/collab/src/tests/integration_tests.rs b/crates/collab/src/tests/integration_tests.rs index d9644856cd..35b6f2c100 100644 --- a/crates/collab/src/tests/integration_tests.rs +++ b/crates/collab/src/tests/integration_tests.rs @@ -7891,11 +7891,11 @@ async fn test_mutual_editor_inlay_hint_cache_update( editor_a.update(cx_a, |_, cx| cx.focus(&editor_a)); cx_a.foreground().run_until_parked(); editor_a.update(cx_a, |editor, _| { - let inlay_cache = editor.inlay_hint_cache().snapshot(); assert!( - inlay_cache.hints.is_empty(), + extract_hint_labels(editor).is_empty(), "No inlays should be in the new cache" ); + 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" @@ -7918,11 +7918,11 @@ async fn test_mutual_editor_inlay_hint_cache_update( editor_b.update(cx_b, |_, cx| cx.focus(&editor_b)); cx_b.foreground().run_until_parked(); editor_b.update(cx_b, |editor, _| { - let inlay_cache = editor.inlay_hint_cache().snapshot(); assert!( - inlay_cache.hints.is_empty(), + extract_hint_labels(editor).is_empty(), "No inlays should be in the new cache" ); + 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" @@ -7978,32 +7978,27 @@ async fn test_mutual_editor_inlay_hint_cache_update( cx_a.foreground().finish_waiting(); cx_a.foreground().run_until_parked(); - fn extract_hint_labels(editor: &Editor) -> Vec<&str> { - editor - .inlay_hint_cache() - .snapshot() - .hints - .iter() - .map(|(_, excerpt_hints)| { - excerpt_hints - .hints - .iter() - .map(|(_, inlay)| match &inlay.label { - project::InlayHintLabel::String(s) => s.as_str(), - _ => unreachable!(), - }) - }) - .flatten() - .collect::>() + fn extract_hint_labels(editor: &Editor) -> Vec { + 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!(), + } + } + } + labels } editor_a.update(cx_a, |editor, _| { assert_eq!( - vec!["0"], + vec!["0".to_string()], extract_hint_labels(editor), "Host should get hints from the 1st edit and 1st LSP query" ); - let inlay_cache = editor.inlay_hint_cache().snapshot(); + let inlay_cache = editor.inlay_hint_cache(); assert_eq!(inlay_cache.allowed_hint_kinds, allowed_hint_kinds, "Inlay kinds settings never change during the test"); assert_eq!( inlay_cache.version, edits_made, @@ -8012,11 +8007,11 @@ async fn test_mutual_editor_inlay_hint_cache_update( }); editor_b.update(cx_b, |editor, _| { assert_eq!( - vec!["0", "1"], + vec!["0".to_string(), "1".to_string()], extract_hint_labels(editor), "Guest should get hints the 1st edit and 2nd LSP query" ); - let inlay_cache = editor.inlay_hint_cache().snapshot(); + let inlay_cache = editor.inlay_hint_cache(); assert_eq!(inlay_cache.allowed_hint_kinds, allowed_hint_kinds, "Inlay kinds settings never change during the test"); assert_eq!( inlay_cache.version, edits_made, @@ -8034,12 +8029,12 @@ async fn test_mutual_editor_inlay_hint_cache_update( cx_b.foreground().run_until_parked(); editor_a.update(cx_a, |editor, _| { assert_eq!( - vec!["0", "1", "2"], + vec!["0".to_string(), "1".to_string(), "2".to_string()], extract_hint_labels(editor), "Host should get hints from 3rd edit, 5th LSP query: \ 4th query was made by guest (but not applied) due to cache invalidation logic" ); - let inlay_cache = editor.inlay_hint_cache().snapshot(); + let inlay_cache = editor.inlay_hint_cache(); assert_eq!(inlay_cache.allowed_hint_kinds, allowed_hint_kinds, "Inlay kinds settings never change during the test"); assert_eq!( inlay_cache.version, edits_made, @@ -8048,11 +8043,16 @@ async fn test_mutual_editor_inlay_hint_cache_update( }); editor_b.update(cx_b, |editor, _| { assert_eq!( - vec!["0", "1", "2", "3"], + vec![ + "0".to_string(), + "1".to_string(), + "2".to_string(), + "3".to_string() + ], extract_hint_labels(editor), "Guest should get hints from 3rd edit, 6th LSP query" ); - let inlay_cache = editor.inlay_hint_cache().snapshot(); + let inlay_cache = editor.inlay_hint_cache(); assert_eq!( inlay_cache.allowed_hint_kinds, allowed_hint_kinds, "Inlay kinds settings never change during the test" @@ -8072,11 +8072,17 @@ async fn test_mutual_editor_inlay_hint_cache_update( cx_b.foreground().run_until_parked(); editor_a.update(cx_a, |editor, _| { assert_eq!( - vec!["0", "1", "2", "3", "4"], + vec![ + "0".to_string(), + "1".to_string(), + "2".to_string(), + "3".to_string(), + "4".to_string() + ], extract_hint_labels(editor), "Host should react to /refresh LSP request and get new hints from 7th LSP query" ); - let inlay_cache = editor.inlay_hint_cache().snapshot(); + let inlay_cache = editor.inlay_hint_cache(); assert_eq!( inlay_cache.allowed_hint_kinds, allowed_hint_kinds, "Inlay kinds settings never change during the test" @@ -8088,11 +8094,11 @@ async fn test_mutual_editor_inlay_hint_cache_update( }); editor_b.update(cx_b, |editor, _| { assert_eq!( - vec!["0", "1", "2", "3", "4", "5"], + vec!["0".to_string(), "1".to_string(), "2".to_string(), "3".to_string(), "4".to_string(), "5".to_string()], extract_hint_labels(editor), "Guest should get a /refresh LSP request propagated by host and get new hints from 8th LSP query" ); - let inlay_cache = editor.inlay_hint_cache().snapshot(); + let inlay_cache = editor.inlay_hint_cache(); assert_eq!( inlay_cache.allowed_hint_kinds, allowed_hint_kinds, "Inlay kinds settings never change during the test" diff --git a/crates/editor/src/inlay_hint_cache.rs b/crates/editor/src/inlay_hint_cache.rs index d94a392d53..aa10807b04 100644 --- a/crates/editor/src/inlay_hint_cache.rs +++ b/crates/editor/src/inlay_hint_cache.rs @@ -9,13 +9,16 @@ use clock::Global; use gpui::{ModelHandle, Task, ViewContext}; use language::{Buffer, BufferSnapshot}; use log::error; +use parking_lot::RwLock; use project::{InlayHint, InlayHintKind}; use collections::{hash_map, HashMap, HashSet}; use util::post_inc; pub struct InlayHintCache { - snapshot: CacheSnapshot, + pub hints: HashMap>>, + pub allowed_hint_kinds: HashSet>, + pub version: usize, update_tasks: HashMap, } @@ -24,13 +27,6 @@ struct InlayHintUpdateTask { _task: Task<()>, } -#[derive(Debug)] -pub struct CacheSnapshot { - pub hints: HashMap>, - pub allowed_hint_kinds: HashSet>, - pub version: usize, -} - #[derive(Debug)] pub struct CachedExcerptHints { version: usize, @@ -84,19 +80,13 @@ struct ExcerptHintsUpdate { impl InlayHintCache { pub fn new(inlay_hint_settings: editor_settings::InlayHints) -> Self { Self { - snapshot: CacheSnapshot { - allowed_hint_kinds: allowed_hint_types(inlay_hint_settings), - hints: HashMap::default(), - version: 0, - }, + allowed_hint_kinds: allowed_hint_types(inlay_hint_settings), + hints: HashMap::default(), update_tasks: HashMap::default(), + version: 0, } } - pub fn snapshot(&self) -> &CacheSnapshot { - &self.snapshot - } - pub fn update_settings( &mut self, multi_buffer: &ModelHandle, @@ -106,37 +96,33 @@ impl InlayHintCache { ) -> Option { let new_allowed_hint_kinds = allowed_hint_types(inlay_hint_settings); if !inlay_hint_settings.enabled { - if self.snapshot.hints.is_empty() { - self.snapshot.allowed_hint_kinds = new_allowed_hint_kinds; + if self.hints.is_empty() { + self.allowed_hint_kinds = new_allowed_hint_kinds; + None } else { self.clear(); - self.snapshot.allowed_hint_kinds = new_allowed_hint_kinds; - return Some(InlaySplice { + self.allowed_hint_kinds = new_allowed_hint_kinds; + Some(InlaySplice { to_remove: visible_hints.iter().map(|inlay| inlay.id).collect(), to_insert: Vec::new(), - }); + }) } - - return None; + } else if new_allowed_hint_kinds == self.allowed_hint_kinds { + None + } else { + let new_splice = self.new_allowed_hint_kinds_splice( + multi_buffer, + &visible_hints, + &new_allowed_hint_kinds, + cx, + ); + if new_splice.is_some() { + self.version += 1; + self.update_tasks.clear(); + self.allowed_hint_kinds = new_allowed_hint_kinds; + } + new_splice } - - if new_allowed_hint_kinds == self.snapshot.allowed_hint_kinds { - return None; - } - - let new_splice = new_allowed_hint_kinds_splice( - &self.snapshot, - multi_buffer, - &visible_hints, - &new_allowed_hint_kinds, - cx, - ); - if new_splice.is_some() { - self.snapshot.version += 1; - self.update_tasks.clear(); - self.snapshot.allowed_hint_kinds = new_allowed_hint_kinds; - } - new_splice } pub fn spawn_hints_update( @@ -154,9 +140,10 @@ impl InlayHintCache { update_tasks .retain(|task_excerpt_id, _| excerpts_to_query.contains_key(task_excerpt_id)); } + 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().version.cmp(&self.snapshot.version) { + hash_map::Entry::Occupied(o) => match o.get().version.cmp(&cache_version) { cmp::Ordering::Less => true, cmp::Ordering::Equal => invalidate_cache, cmp::Ordering::Greater => false, @@ -169,7 +156,6 @@ impl InlayHintCache { update_tasks .retain(|task_excerpt_id, _| excerpts_to_query.contains_key(task_excerpt_id)); } - let cache_version = self.snapshot.version; excerpts_to_query.retain(|visible_excerpt_id, _| { match update_tasks.entry(*visible_excerpt_id) { hash_map::Entry::Occupied(o) => match o.get().version.cmp(&cache_version) { @@ -211,15 +197,12 @@ impl InlayHintCache { cache_version, invalidate, }; - let cached_excxerpt_hints = editor - .inlay_hint_cache - .snapshot - .hints - .get(&excerpt_id) - .cloned(); + 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; @@ -250,11 +233,113 @@ impl InlayHintCache { .detach(); } + fn new_allowed_hint_kinds_splice( + &self, + multi_buffer: &ModelHandle, + visible_hints: &[Inlay], + new_kinds: &HashSet>, + cx: &mut ViewContext, + ) -> Option { + let old_kinds = &self.allowed_hint_kinds; + if new_kinds == old_kinds { + return None; + } + + let mut to_remove = Vec::new(); + let mut to_insert = Vec::new(); + let mut shown_hints_to_remove = visible_hints.iter().fold( + HashMap::>::default(), + |mut current_hints, inlay| { + current_hints + .entry(inlay.position.excerpt_id) + .or_default() + .push((inlay.position, inlay.id)); + current_hints + }, + ); + + let multi_buffer = multi_buffer.read(cx); + let multi_buffer_snapshot = multi_buffer.snapshot(cx); + + for (excerpt_id, excerpt_cached_hints) in &self.hints { + let shown_excerpt_hints_to_remove = + shown_hints_to_remove.entry(*excerpt_id).or_default(); + let excerpt_cached_hints = excerpt_cached_hints.read(); + let mut excerpt_cache = excerpt_cached_hints.hints.iter().fuse().peekable(); + shown_excerpt_hints_to_remove.retain(|(shown_anchor, shown_hint_id)| { + let Some(buffer) = shown_anchor + .buffer_id + .and_then(|buffer_id| multi_buffer.buffer(buffer_id)) else { return false }; + let buffer_snapshot = buffer.read(cx).snapshot(); + loop { + match excerpt_cache.peek() { + Some((cached_hint_id, cached_hint)) => { + if cached_hint_id == shown_hint_id { + excerpt_cache.next(); + return !new_kinds.contains(&cached_hint.kind); + } + + match cached_hint + .position + .cmp(&shown_anchor.text_anchor, &buffer_snapshot) + { + cmp::Ordering::Less | cmp::Ordering::Equal => { + if !old_kinds.contains(&cached_hint.kind) + && new_kinds.contains(&cached_hint.kind) + { + to_insert.push(( + multi_buffer_snapshot.anchor_in_excerpt( + *excerpt_id, + cached_hint.position, + ), + *cached_hint_id, + cached_hint.clone(), + )); + } + excerpt_cache.next(); + } + cmp::Ordering::Greater => return true, + } + } + None => return true, + } + } + }); + + for (cached_hint_id, maybe_missed_cached_hint) in excerpt_cache { + let cached_hint_kind = maybe_missed_cached_hint.kind; + if !old_kinds.contains(&cached_hint_kind) && new_kinds.contains(&cached_hint_kind) { + to_insert.push(( + multi_buffer_snapshot + .anchor_in_excerpt(*excerpt_id, maybe_missed_cached_hint.position), + *cached_hint_id, + maybe_missed_cached_hint.clone(), + )); + } + } + } + + to_remove.extend( + shown_hints_to_remove + .into_values() + .flatten() + .map(|(_, hint_id)| hint_id), + ); + if to_remove.is_empty() && to_insert.is_empty() { + None + } else { + Some(InlaySplice { + to_remove, + to_insert, + }) + } + } + fn clear(&mut self) { - self.snapshot.version += 1; + self.version += 1; self.update_tasks.clear(); - self.snapshot.hints.clear(); - self.snapshot.allowed_hint_kinds.clear(); + self.hints.clear(); + self.allowed_hint_kinds.clear(); } } @@ -263,7 +348,7 @@ fn new_update_task( multi_buffer_snapshot: MultiBufferSnapshot, buffer_snapshot: BufferSnapshot, visible_hints: Arc>, - cached_excerpt_hints: Option>, + cached_excerpt_hints: Option>>, cx: &mut ViewContext<'_, '_, Editor>, ) -> InlayHintUpdateTask { let hints_fetch_task = hints_fetch_task(query, cx); @@ -290,19 +375,16 @@ fn new_update_task( .update(&mut cx, |editor, cx| { let cached_excerpt_hints = editor .inlay_hint_cache - .snapshot .hints .entry(new_update.excerpt_id) .or_insert_with(|| { - Arc::new(CachedExcerptHints { + Arc::new(RwLock::new(CachedExcerptHints { version: new_update.cache_version, buffer_version: buffer_snapshot.version().clone(), hints: Vec::new(), - }) + })) }); - let cached_excerpt_hints = Arc::get_mut(cached_excerpt_hints) - .expect("Cached excerpt hints were dropped with the task"); - + 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 => { @@ -314,7 +396,7 @@ fn new_update_task( }); cached_excerpt_hints.buffer_version = buffer_snapshot.version().clone(); - editor.inlay_hint_cache.snapshot.version += 1; + editor.inlay_hint_cache.version += 1; let mut splice = InlaySplice { to_remove: new_update.remove_from_visible, @@ -327,7 +409,6 @@ fn new_update_task( let new_inlay_id = InlayId(post_inc(&mut editor.next_inlay_id)); if editor .inlay_hint_cache - .snapshot .allowed_hint_kinds .contains(&new_hint.kind) { @@ -346,6 +427,7 @@ fn new_update_task( .sort_by(|(_, hint_a), (_, hint_b)| { hint_a.position.cmp(&hint_b.position, &buffer_snapshot) }); + drop(cached_excerpt_hints); let InlaySplice { to_remove, @@ -368,109 +450,11 @@ fn new_update_task( } } -fn new_allowed_hint_kinds_splice( - cache: &CacheSnapshot, - multi_buffer: &ModelHandle, - visible_hints: &[Inlay], - new_kinds: &HashSet>, - cx: &mut ViewContext, -) -> Option { - let old_kinds = &cache.allowed_hint_kinds; - if new_kinds == old_kinds { - return None; - } - - let mut to_remove = Vec::new(); - let mut to_insert = Vec::new(); - let mut shown_hints_to_remove = visible_hints.iter().fold( - HashMap::>::default(), - |mut current_hints, inlay| { - current_hints - .entry(inlay.position.excerpt_id) - .or_default() - .push((inlay.position, inlay.id)); - current_hints - }, - ); - - let multi_buffer = multi_buffer.read(cx); - let multi_buffer_snapshot = multi_buffer.snapshot(cx); - - for (excerpt_id, excerpt_cached_hints) in &cache.hints { - let shown_excerpt_hints_to_remove = shown_hints_to_remove.entry(*excerpt_id).or_default(); - let mut excerpt_cache = excerpt_cached_hints.hints.iter().fuse().peekable(); - shown_excerpt_hints_to_remove.retain(|(shown_anchor, shown_hint_id)| { - let Some(buffer) = shown_anchor - .buffer_id - .and_then(|buffer_id| multi_buffer.buffer(buffer_id)) else { return false }; - let buffer_snapshot = buffer.read(cx).snapshot(); - loop { - match excerpt_cache.peek() { - Some((cached_hint_id, cached_hint)) => { - if cached_hint_id == shown_hint_id { - excerpt_cache.next(); - return !new_kinds.contains(&cached_hint.kind); - } - - match cached_hint - .position - .cmp(&shown_anchor.text_anchor, &buffer_snapshot) - { - cmp::Ordering::Less | cmp::Ordering::Equal => { - if !old_kinds.contains(&cached_hint.kind) - && new_kinds.contains(&cached_hint.kind) - { - to_insert.push(( - multi_buffer_snapshot - .anchor_in_excerpt(*excerpt_id, cached_hint.position), - *cached_hint_id, - cached_hint.clone(), - )); - } - excerpt_cache.next(); - } - cmp::Ordering::Greater => return true, - } - } - None => return true, - } - } - }); - - for (cached_hint_id, maybe_missed_cached_hint) in excerpt_cache { - let cached_hint_kind = maybe_missed_cached_hint.kind; - if !old_kinds.contains(&cached_hint_kind) && new_kinds.contains(&cached_hint_kind) { - to_insert.push(( - multi_buffer_snapshot - .anchor_in_excerpt(*excerpt_id, maybe_missed_cached_hint.position), - *cached_hint_id, - maybe_missed_cached_hint.clone(), - )); - } - } - } - - to_remove.extend( - shown_hints_to_remove - .into_values() - .flatten() - .map(|(_, hint_id)| hint_id), - ); - if to_remove.is_empty() && to_insert.is_empty() { - None - } else { - Some(InlaySplice { - to_remove, - to_insert, - }) - } -} - fn new_excerpt_hints_update_result( query: ExcerptQuery, new_excerpt_hints: Vec, buffer_snapshot: &BufferSnapshot, - cached_excerpt_hints: Option>, + cached_excerpt_hints: Option>>, visible_hints: &[Inlay], ) -> Option { let mut add_to_cache: Vec = Vec::new(); @@ -482,6 +466,7 @@ fn new_excerpt_hints_update_result( } let missing_from_cache = match &cached_excerpt_hints { Some(cached_excerpt_hints) => { + let cached_excerpt_hints = cached_excerpt_hints.read(); match cached_excerpt_hints.hints.binary_search_by(|probe| { probe.1.position.cmp(&new_hint.position, buffer_snapshot) }) { @@ -518,15 +503,19 @@ fn new_excerpt_hints_update_result( .map(|inlay_hint| inlay_hint.id) .filter(|hint_id| !excerpt_hints_to_persist.contains_key(hint_id)), ); - remove_from_cache.extend( - cached_excerpt_hints - .iter() - .flat_map(|excerpt_hints| excerpt_hints.hints.iter()) - .filter(|(cached_inlay_id, _)| { - !excerpt_hints_to_persist.contains_key(cached_inlay_id) - }) - .map(|(cached_inlay_id, _)| *cached_inlay_id), - ); + + if let Some(cached_excerpt_hints) = &cached_excerpt_hints { + let cached_excerpt_hints = cached_excerpt_hints.read(); + remove_from_cache.extend( + cached_excerpt_hints + .hints + .iter() + .filter(|(cached_inlay_id, _)| { + !excerpt_hints_to_persist.contains_key(cached_inlay_id) + }) + .map(|(cached_inlay_id, _)| *cached_inlay_id), + ); + } } if remove_from_visible.is_empty() && remove_from_cache.is_empty() && add_to_cache.is_empty() {