From 7abaf22b93ec892947e80f9e2bb179f8d6c046df Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Tue, 13 Jun 2023 01:46:03 +0300 Subject: [PATCH] Generate proper inlay diffs for splice --- crates/editor/src/display_map.rs | 26 +-- crates/editor/src/display_map/inlay_map.rs | 96 +++++----- crates/editor/src/editor.rs | 64 +++++-- crates/editor/src/inlay_cache.rs | 204 +++++++++++++++++++++ crates/editor/src/inlay_hint_storage.rs | 46 ----- 5 files changed, 317 insertions(+), 119 deletions(-) create mode 100644 crates/editor/src/inlay_cache.rs delete mode 100644 crates/editor/src/inlay_hint_storage.rs diff --git a/crates/editor/src/display_map.rs b/crates/editor/src/display_map.rs index 40f6ecb76ee3af8131fa210c3cc4bc4f34017ec9..d8924f9692505997c6dd9b1d8d66dae07d80c3f0 100644 --- a/crates/editor/src/display_map.rs +++ b/crates/editor/src/display_map.rs @@ -6,8 +6,8 @@ mod tab_map; mod wrap_map; use crate::{ - display_map::inlay_map::InlayProperties, Anchor, AnchorRangeExt, MultiBuffer, - MultiBufferSnapshot, ToOffset, ToPoint, + display_map::inlay_map::InlayProperties, inlay_cache::InlayId, Anchor, AnchorRangeExt, + MultiBuffer, MultiBufferSnapshot, ToOffset, ToPoint, }; pub use block_map::{BlockMap, BlockPoint}; use collections::{HashMap, HashSet}; @@ -287,7 +287,8 @@ impl DisplayMap { pub fn splice_inlays( &mut self, - new_hints: Vec<(Anchor, project::InlayHint)>, + to_remove: Vec, + to_insert: Vec<(InlayId, Anchor, project::InlayHint)>, cx: &mut ModelContext, ) { let buffer_snapshot = self.buffer.read(cx).snapshot(cx); @@ -302,18 +303,19 @@ impl DisplayMap { .update(cx, |map, cx| map.sync(snapshot, edits, cx)); self.block_map.read(snapshot, edits); - let new_inlays = new_hints + let new_inlays = to_insert .into_iter() - .map(|(hint_anchor, hint)| InlayProperties { - position: hint_anchor.bias_left(&buffer_snapshot), - text: hint.text(), + .map(|(inlay_id, hint_anchor, hint)| { + ( + inlay_id, + InlayProperties { + position: hint_anchor.bias_left(&buffer_snapshot), + text: hint.text(), + }, + ) }) .collect(); - let (snapshot, edits, _) = self.inlay_map.splice( - // TODO kb this is wrong, calc diffs in the editor instead. - self.inlay_map.inlays.keys().copied().collect(), - new_inlays, - ); + let (snapshot, edits) = self.inlay_map.splice(to_remove, new_inlays); let (snapshot, edits) = self.tab_map.sync(snapshot, edits, tab_size); let (snapshot, edits) = self .wrap_map diff --git a/crates/editor/src/display_map/inlay_map.rs b/crates/editor/src/display_map/inlay_map.rs index e2308bfcc9fabced9ecf7b8822bd1d0c58efef85..216436bb11d8a73f924b6be79da1315c1574ca7a 100644 --- a/crates/editor/src/display_map/inlay_map.rs +++ b/crates/editor/src/display_map/inlay_map.rs @@ -5,8 +5,8 @@ use super::{ }, TextHighlights, }; -use crate::{Anchor, MultiBufferSnapshot, ToPoint}; -use collections::{BTreeMap, HashMap, HashSet}; +use crate::{inlay_cache::InlayId, Anchor, MultiBufferSnapshot, ToPoint}; +use collections::{BTreeMap, HashMap}; use gpui::fonts::HighlightStyle; use language::{Chunk, Edit, Point, Rope, TextSummary}; use parking_lot::Mutex; @@ -16,14 +16,9 @@ use std::{ }; use sum_tree::{Bias, Cursor, SumTree}; use text::Patch; -use util::post_inc; - -#[derive(Clone, Copy, Debug, Default, PartialEq, Eq, PartialOrd, Ord, Hash)] -pub struct InlayId(usize); pub struct InlayMap { snapshot: Mutex, - next_inlay_id: usize, pub(super) inlays: HashMap, } @@ -247,7 +242,6 @@ impl InlayMap { ( Self { snapshot: Mutex::new(snapshot.clone()), - next_inlay_id: 0, inlays: HashMap::default(), }, snapshot, @@ -372,21 +366,19 @@ impl InlayMap { pub fn splice>( &mut self, - to_remove: HashSet, - to_insert: Vec>, - ) -> (InlaySnapshot, Vec, Vec) { + to_remove: Vec, + to_insert: Vec<(InlayId, InlayProperties)>, + ) -> (InlaySnapshot, Vec) { let mut snapshot = self.snapshot.lock(); let mut inlays = BTreeMap::new(); - let mut new_ids = Vec::new(); - for properties in to_insert { + for (id, properties) in to_insert { let inlay = Inlay { - id: InlayId(post_inc(&mut self.next_inlay_id)), + id, position: properties.position, text: properties.text.into(), }; self.inlays.insert(inlay.id, inlay.clone()); - new_ids.push(inlay.id); let buffer_point = inlay.position.to_point(snapshot.buffer_snapshot()); let fold_point = snapshot @@ -501,20 +493,20 @@ impl InlayMap { snapshot.version += 1; snapshot.check_invariants(); - (snapshot.clone(), inlay_edits.into_inner(), new_ids) + (snapshot.clone(), inlay_edits.into_inner()) } #[cfg(any(test, feature = "test-support"))] pub fn randomly_mutate( &mut self, rng: &mut rand::rngs::StdRng, - ) -> (InlaySnapshot, Vec, Vec) { + ) -> (InlaySnapshot, Vec) { use rand::prelude::*; - let mut to_remove = HashSet::default(); - let mut to_insert = Vec::default(); + let mut to_remove = Vec::new(); + let mut to_insert = Vec::new(); let snapshot = self.snapshot.lock(); - for _ in 0..rng.gen_range(1..=5) { + for i in 0..rng.gen_range(1..=5) { if self.inlays.is_empty() || rng.gen() { let buffer_snapshot = snapshot.buffer_snapshot(); let position = buffer_snapshot.random_byte_range(0, rng).start; @@ -529,12 +521,15 @@ impl InlayMap { bias, text ); - to_insert.push(InlayProperties { - position: buffer_snapshot.anchor_at(position, bias), - text, - }); + to_insert.push(( + InlayId(i), + InlayProperties { + position: buffer_snapshot.anchor_at(position, bias), + text, + }, + )); } else { - to_remove.insert(*self.inlays.keys().choose(rng).unwrap()); + to_remove.push(*self.inlays.keys().choose(rng).unwrap()); } } @@ -841,6 +836,7 @@ mod tests { use settings::SettingsStore; use std::env; use text::Patch; + use util::post_inc; #[gpui::test] fn test_basic_inlays(cx: &mut AppContext) { @@ -850,13 +846,17 @@ mod tests { let (suggestion_map, suggestion_snapshot) = SuggestionMap::new(fold_snapshot.clone()); let (mut inlay_map, inlay_snapshot) = InlayMap::new(suggestion_snapshot.clone()); assert_eq!(inlay_snapshot.text(), "abcdefghi"); + let mut next_inlay_id = 0; - let (inlay_snapshot, _, _) = inlay_map.splice( - HashSet::default(), - vec![InlayProperties { - position: buffer.read(cx).snapshot(cx).anchor_after(3), - text: "|123|", - }], + let (inlay_snapshot, _) = inlay_map.splice( + Vec::new(), + vec![( + InlayId(post_inc(&mut next_inlay_id)), + InlayProperties { + position: buffer.read(cx).snapshot(cx).anchor_after(3), + text: "|123|", + }, + )], ); assert_eq!(inlay_snapshot.text(), "abc|123|defghi"); assert_eq!( @@ -932,17 +932,23 @@ mod tests { let (inlay_snapshot, _) = inlay_map.sync(suggestion_snapshot.clone(), suggestion_edits); assert_eq!(inlay_snapshot.text(), "abxyDzefghi"); - let (inlay_snapshot, _, inlay_ids) = inlay_map.splice( - HashSet::default(), + let (inlay_snapshot, _) = inlay_map.splice( + Vec::new(), vec![ - InlayProperties { - position: buffer.read(cx).snapshot(cx).anchor_before(3), - text: "|123|", - }, - InlayProperties { - position: buffer.read(cx).snapshot(cx).anchor_after(3), - text: "|456|", - }, + ( + InlayId(post_inc(&mut next_inlay_id)), + InlayProperties { + position: buffer.read(cx).snapshot(cx).anchor_before(3), + text: "|123|", + }, + ), + ( + InlayId(post_inc(&mut next_inlay_id)), + InlayProperties { + position: buffer.read(cx).snapshot(cx).anchor_after(3), + text: "|456|", + }, + ), ], ); assert_eq!(inlay_snapshot.text(), "abx|123||456|yDzefghi"); @@ -959,8 +965,8 @@ mod tests { assert_eq!(inlay_snapshot.text(), "abx|123|JKL|456|yDzefghi"); // The inlays can be manually removed. - let (inlay_snapshot, _, _) = - inlay_map.splice::(HashSet::from_iter(inlay_ids), Default::default()); + let (inlay_snapshot, _) = + inlay_map.splice::(inlay_map.inlays.keys().copied().collect(), Vec::new()); assert_eq!(inlay_snapshot.text(), "abxJKLyDzefghi"); } @@ -996,8 +1002,8 @@ mod tests { let mut buffer_edits = Vec::new(); match rng.gen_range(0..=100) { 0..=29 => { - let (snapshot, edits, _) = inlay_map.randomly_mutate(&mut rng); - inlay_snapshot = snapshot; + let (snapshot, edits) = inlay_map.randomly_mutate(&mut rng); + log::info!("mutated text: {:?}", snapshot.text()); inlay_edits = Patch::new(edits); } 30..=59 => { diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index cd12dbefd163de9ebbe8988c44ed35e134e3d539..fac7d4033b9089588eba6561600cb82867000797 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -2,7 +2,7 @@ mod blink_manager; pub mod display_map; mod editor_settings; mod element; -mod inlay_hint_storage; +mod inlay_cache; mod git; mod highlight_matching_bracket; @@ -26,8 +26,8 @@ use aho_corasick::AhoCorasick; use anyhow::{anyhow, Context, Result}; use blink_manager::BlinkManager; use client::{ClickhouseEvent, TelemetrySettings}; -use clock::ReplicaId; -use collections::{BTreeMap, Bound, HashMap, HashSet, VecDeque}; +use clock::{Global, ReplicaId}; +use collections::{hash_map, BTreeMap, Bound, HashMap, HashSet, VecDeque}; use copilot::Copilot; pub use display_map::DisplayPoint; use display_map::*; @@ -53,7 +53,7 @@ use gpui::{ }; use highlight_matching_bracket::refresh_matching_bracket_highlights; use hover_popover::{hide_hover, HoverState}; -use inlay_hint_storage::InlayHintStorage; +use inlay_cache::{InlayCache, InlaysUpdate, OrderedByAnchorOffset}; pub use items::MAX_TAB_TITLE_LEN; use itertools::Itertools; pub use language::{char_kind, CharKind}; @@ -540,7 +540,7 @@ pub struct Editor { gutter_hovered: bool, link_go_to_definition_state: LinkGoToDefinitionState, copilot_state: CopilotState, - inlay_hint_storage: InlayHintStorage, + inlay_hint_cache: InlayCache, _subscriptions: Vec, } @@ -1355,7 +1355,7 @@ impl Editor { hover_state: Default::default(), link_go_to_definition_state: Default::default(), copilot_state: Default::default(), - inlay_hint_storage: InlayHintStorage::default(), + inlay_hint_cache: InlayCache::default(), gutter_hovered: false, _subscriptions: vec![ cx.observe(&buffer, Self::on_buffer_changed), @@ -2604,7 +2604,14 @@ impl Editor { // TODO kb every time I reopen the same buffer, it's different. // Find a way to understand it's the same buffer. Use paths? let buffer_id = buffer_snapshot.remote_id(); + let buffer_version = buffer_snapshot.version().clone(); let buffer_handle = multi_buffer.read(cx).buffer(buffer_id)?; + if self + .inlay_hint_cache + .inlays_up_to_date(buffer_id, &buffer_version, excerpt_id) + { + return None; + } let task = cx.spawn(|editor, mut cx| async move { let task = editor @@ -2622,6 +2629,8 @@ impl Editor { .context("inlay hints fecth task spawn")?; anyhow::Ok(( + buffer_id, + buffer_version, excerpt_id, match task { Some(task) => task.await.context("inlay hints for buffer task")?, @@ -2634,29 +2643,52 @@ impl Editor { .collect::>(); cx.spawn(|editor, mut cx| async move { - let mut hints_to_draw: Vec<(Anchor, InlayHint)> = Vec::new(); + let mut hints_response: HashMap< + u64, + (Global, HashMap>), + > = HashMap::default(); let multi_buffer_snapshot = editor.read_with(&cx, |editor, cx| editor.buffer().read(cx).snapshot(cx))?; for task_result in futures::future::join_all(hint_fetch_tasks).await { match task_result { - Ok((excerpt_id, excerpt_hints)) => { - if !excerpt_hints.is_empty() { - hints_to_draw.extend(excerpt_hints.into_iter().map(|hint| { - let anchor = multi_buffer_snapshot - .anchor_in_excerpt(excerpt_id, hint.position); - (anchor, hint) - })); + Ok((buffer_id, buffer_version, excerpt_id, excerpt_hints)) => { + let excerpt_hints_response = HashMap::from_iter([( + excerpt_id, + excerpt_hints.into_iter().fold( + OrderedByAnchorOffset::default(), + |mut ordered_hints, hint| { + let anchor = multi_buffer_snapshot + .anchor_in_excerpt(excerpt_id, hint.position); + ordered_hints.add(anchor, hint); + ordered_hints + }, + ), + )]); + match hints_response.entry(buffer_id) { + hash_map::Entry::Occupied(mut o) => { + o.get_mut().1.extend(excerpt_hints_response); + } + hash_map::Entry::Vacant(v) => { + v.insert((buffer_version, excerpt_hints_response)); + } } } Err(e) => error!("Failed to update hints for buffer: {e:#}"), } } - if !hints_to_draw.is_empty() { + if !hints_response.is_empty() { + let InlaysUpdate { + to_remove, + to_insert, + } = editor.update(&mut cx, |editor, _| { + editor.inlay_hint_cache.update_inlays(hints_response) + })?; + editor.update(&mut cx, |editor, cx| { editor.display_map.update(cx, |display_map, cx| { - display_map.splice_inlays(hints_to_draw, cx); + display_map.splice_inlays(to_remove, to_insert, cx); }); })?; } diff --git a/crates/editor/src/inlay_cache.rs b/crates/editor/src/inlay_cache.rs new file mode 100644 index 0000000000000000000000000000000000000000..e5c48dcba2f7f7350956680bf4df54f0aa13f510 --- /dev/null +++ b/crates/editor/src/inlay_cache.rs @@ -0,0 +1,204 @@ +use std::cmp; + +use crate::{Anchor, ExcerptId}; +use clock::Global; +use project::InlayHint; +use util::post_inc; + +use collections::{BTreeMap, HashMap}; + +#[derive(Clone, Debug, Default)] +pub struct InlayCache { + inlays_per_buffer: HashMap, + next_inlay_id: usize, +} + +#[derive(Clone, Debug)] +pub struct OrderedByAnchorOffset(pub BTreeMap); + +impl OrderedByAnchorOffset { + pub fn add(&mut self, anchor: Anchor, t: T) { + self.0.insert(anchor.text_anchor.offset, (anchor, t)); + } + + fn into_ordered_elements(self) -> impl Iterator { + self.0.into_values() + } +} + +impl Default for OrderedByAnchorOffset { + fn default() -> Self { + Self(BTreeMap::default()) + } +} + +#[derive(Clone, Debug, Default)] +struct BufferInlays { + buffer_version: Global, + inlays_per_excerpts: HashMap>, +} + +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq, PartialOrd, Ord, Hash)] +pub struct InlayId(pub usize); + +#[derive(Debug)] +pub struct InlaysUpdate { + pub to_remove: Vec, + pub to_insert: Vec<(InlayId, Anchor, InlayHint)>, +} + +impl InlayCache { + pub fn inlays_up_to_date( + &self, + buffer_id: u64, + buffer_version: &Global, + excerpt_id: ExcerptId, + ) -> bool { + let Some(buffer_inlays) = self.inlays_per_buffer.get(&buffer_id) else { return false }; + let buffer_up_to_date = buffer_version == &buffer_inlays.buffer_version + || buffer_inlays.buffer_version.changed_since(buffer_version); + buffer_up_to_date && buffer_inlays.inlays_per_excerpts.contains_key(&excerpt_id) + } + + pub fn update_inlays( + &mut self, + new_inlays: HashMap>)>, + ) -> InlaysUpdate { + let mut old_inlays = self.inlays_per_buffer.clone(); + let mut to_remove = Vec::new(); + let mut to_insert = Vec::new(); + + for (buffer_id, (buffer_version, new_buffer_inlays)) in new_inlays { + match old_inlays.remove(&buffer_id) { + Some(mut old_buffer_inlays) => { + for (excerpt_id, new_excerpt_inlays) in new_buffer_inlays { + if self.inlays_up_to_date(buffer_id, &buffer_version, excerpt_id) { + continue; + } + + let self_inlays_per_buffer = self + .inlays_per_buffer + .get_mut(&buffer_id) + .expect("element expected: `old_inlays.remove` returned `Some`"); + let mut new_excerpt_inlays = + new_excerpt_inlays.into_ordered_elements().fuse().peekable(); + if old_buffer_inlays + .inlays_per_excerpts + .remove(&excerpt_id) + .is_some() + { + let self_excerpt_inlays = self_inlays_per_buffer + .inlays_per_excerpts + .get_mut(&excerpt_id) + .expect("element expected: `old_excerpt_inlays` is `Some`"); + let mut hints_to_add = Vec::<(Anchor, (InlayId, InlayHint))>::new(); + self_excerpt_inlays.0.retain( + |_, (old_anchor, (old_inlay_id, old_inlay))| { + let mut retain = false; + + while let Some(new_offset) = new_excerpt_inlays + .peek() + .map(|(new_anchor, _)| new_anchor.text_anchor.offset) + { + let old_offset = old_anchor.text_anchor.offset; + match new_offset.cmp(&old_offset) { + cmp::Ordering::Less => { + let (new_anchor, new_inlay) = + new_excerpt_inlays.next().expect( + "element expected: `peek` returned `Some`", + ); + hints_to_add.push(( + new_anchor, + ( + InlayId(post_inc(&mut self.next_inlay_id)), + new_inlay, + ), + )); + } + cmp::Ordering::Equal => { + let (new_anchor, new_inlay) = + new_excerpt_inlays.next().expect( + "element expected: `peek` returned `Some`", + ); + if &new_inlay == old_inlay { + retain = true; + } else { + hints_to_add.push(( + new_anchor, + ( + InlayId(post_inc( + &mut self.next_inlay_id, + )), + new_inlay, + ), + )); + } + } + cmp::Ordering::Greater => break, + } + } + + if !retain { + to_remove.push(*old_inlay_id); + } + retain + }, + ); + + for (new_anchor, (id, new_inlay)) in hints_to_add { + self_excerpt_inlays.add(new_anchor, (id, new_inlay.clone())); + to_insert.push((id, new_anchor, new_inlay)); + } + } + + for (new_anchor, new_inlay) in new_excerpt_inlays { + let id = InlayId(post_inc(&mut self.next_inlay_id)); + self_inlays_per_buffer + .inlays_per_excerpts + .entry(excerpt_id) + .or_default() + .add(new_anchor, (id, new_inlay.clone())); + to_insert.push((id, new_anchor, new_inlay)); + } + } + } + None => { + let mut inlays_per_excerpts: HashMap< + ExcerptId, + OrderedByAnchorOffset<(InlayId, InlayHint)>, + > = HashMap::default(); + for (new_excerpt_id, new_ordered_inlays) in new_buffer_inlays { + for (new_anchor, new_inlay) in new_ordered_inlays.into_ordered_elements() { + let id = InlayId(post_inc(&mut self.next_inlay_id)); + inlays_per_excerpts + .entry(new_excerpt_id) + .or_default() + .add(new_anchor, (id, new_inlay.clone())); + to_insert.push((id, new_anchor, new_inlay)); + } + } + self.inlays_per_buffer.insert( + buffer_id, + BufferInlays { + buffer_version, + inlays_per_excerpts, + }, + ); + } + } + } + + for (_, old_buffer_inlays) in old_inlays { + for (_, old_excerpt_inlays) in old_buffer_inlays.inlays_per_excerpts { + for (_, (id_to_remove, _)) in old_excerpt_inlays.into_ordered_elements() { + to_remove.push(id_to_remove); + } + } + } + + InlaysUpdate { + to_remove, + to_insert, + } + } +} diff --git a/crates/editor/src/inlay_hint_storage.rs b/crates/editor/src/inlay_hint_storage.rs deleted file mode 100644 index fcb89fa9138863e97459de5a6ca03f63797408e7..0000000000000000000000000000000000000000 --- a/crates/editor/src/inlay_hint_storage.rs +++ /dev/null @@ -1,46 +0,0 @@ -use crate::Anchor; -use project::InlayHint; - -use collections::BTreeMap; - -#[derive(Debug, Default)] -pub struct InlayHintStorage { - hints: BTreeMap, -} - -impl InlayHintStorage { - fn insert(&mut self) -> bool { - todo!("TODO kb") - } -} -// TODO kb need to understand different inlay hint update cases: -// * new hints from the new excerpt (no need to invalidate the cache) -// * new hints after /refresh or a text edit (whole cache should be purged) -// ??? revert/reopened files could get a speedup, if we don't truly delete the hints, but hide them in another var? - -// let buffer_version = -// cx.read(|cx| buffer.read(cx).version().clone()); - -// #[derive(Debug, Default, Clone)] -// struct InlayHintVersions { -// last_buffer_versions_with_hints: HashMap, -// } - -// impl InlayHintVersions { -// fn absent_or_newer(&self, location: &InlayHintLocation, new_version: &Global) -> bool { -// self.last_buffer_versions_with_hints -// .get(location) -// .map(|last_version_with_hints| new_version.changed_since(&last_version_with_hints)) -// .unwrap_or(true) -// } - -// fn insert(&mut self, location: InlayHintLocation, new_version: Global) -> bool { -// if self.absent_or_newer(&location, &new_version) { -// self.last_buffer_versions_with_hints -// .insert(location, new_version); -// true -// } else { -// false -// } -// } -// }