From ad11d83724311ae75baa1150d2a4ee09e679fa79 Mon Sep 17 00:00:00 2001 From: Piotr Osiewicz <24362066+osiewicz@users.noreply.github.com> Date: Fri, 2 Aug 2024 19:51:26 +0200 Subject: [PATCH] Skip over folded regions when iterating over multibuffer chunks (#15646) This commit weaves through new APIs for language::BufferChunks, multi_buffer::MultiBufferChunks and inlay_map::InlayChunks that allow seeking with an upper-bound. This allows us to omit doing syntax highligting and looking up diagnostics for folded ranges. This in turn directly improves performance of assistant panel with large contexts. Release Notes: - Fixed poor performance when editing in the assistant panel after inserting large files using slash commands --------- Co-authored-by: Max Co-authored-by: Max Brunsfeld --- Cargo.lock | 1 + crates/editor/Cargo.toml | 1 + crates/editor/src/display_map/fold_map.rs | 137 ++++++++++++++------- crates/editor/src/display_map/inlay_map.rs | 12 +- crates/editor/src/editor_tests.rs | 57 +++++++++ crates/language/src/buffer.rs | 124 ++++++++++++------- crates/language/src/language.rs | 2 +- crates/multi_buffer/src/multi_buffer.rs | 28 +++-- crates/rope/src/rope.rs | 5 + 9 files changed, 261 insertions(+), 106 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 56e8f1911e..fa345fe052 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3599,6 +3599,7 @@ dependencies = [ "time", "time_format", "tree-sitter-html", + "tree-sitter-md", "tree-sitter-rust", "tree-sitter-typescript", "ui", diff --git a/crates/editor/Cargo.toml b/crates/editor/Cargo.toml index 8fb4e3be32..7eb3eb710e 100644 --- a/crates/editor/Cargo.toml +++ b/crates/editor/Cargo.toml @@ -93,6 +93,7 @@ settings = { workspace = true, features = ["test-support"] } text = { workspace = true, features = ["test-support"] } theme = { workspace = true, features = ["test-support"] } tree-sitter-html.workspace = true +tree-sitter-md.workspace = true tree-sitter-rust.workspace = true tree-sitter-typescript.workspace = true unindent.workspace = true diff --git a/crates/editor/src/display_map/fold_map.rs b/crates/editor/src/display_map/fold_map.rs index 328aef9b45..fa0e0ac472 100644 --- a/crates/editor/src/display_map/fold_map.rs +++ b/crates/editor/src/display_map/fold_map.rs @@ -11,7 +11,7 @@ use std::{ ops::{Add, AddAssign, Deref, DerefMut, Range, Sub}, sync::Arc, }; -use sum_tree::{Bias, Cursor, FilterCursor, SumTree}; +use sum_tree::{Bias, Cursor, FilterCursor, SumTree, Summary}; use util::post_inc; #[derive(Clone)] @@ -277,6 +277,17 @@ impl FoldMap { "transform tree does not match inlay snapshot's length" ); + let mut prev_transform_isomorphic = false; + for transform in self.snapshot.transforms.iter() { + if !transform.is_fold() && prev_transform_isomorphic { + panic!( + "found adjacent isomorphic transforms: {:?}", + self.snapshot.transforms.items(&()) + ); + } + prev_transform_isomorphic = !transform.is_fold(); + } + let mut folds = self.snapshot.folds.iter().peekable(); while let Some(fold) = folds.next() { if let Some(next_fold) = folds.peek() { @@ -303,11 +314,24 @@ impl FoldMap { } else { let mut inlay_edits_iter = inlay_edits.iter().cloned().peekable(); - let mut new_transforms = SumTree::new(); + let mut new_transforms = SumTree::::new(); let mut cursor = self.snapshot.transforms.cursor::(); cursor.seek(&InlayOffset(0), Bias::Right, &()); while let Some(mut edit) = inlay_edits_iter.next() { + if let Some(item) = cursor.item() { + if !item.is_fold() { + new_transforms.update_last( + |transform| { + if !transform.is_fold() { + transform.summary.add_summary(&item.summary, &()); + cursor.next(&()); + } + }, + &(), + ); + } + } new_transforms.append(cursor.slice(&edit.old.start, Bias::Left, &()), &()); edit.new.start -= edit.old.start - *cursor.start(); edit.old.start = *cursor.start(); @@ -392,16 +416,7 @@ impl FoldMap { if fold_range.start.0 > sum.input.len { let text_summary = inlay_snapshot .text_summary_for_range(InlayOffset(sum.input.len)..fold_range.start); - new_transforms.push( - Transform { - summary: TransformSummary { - output: text_summary.clone(), - input: text_summary, - }, - placeholder: None, - }, - &(), - ); + push_isomorphic(&mut new_transforms, text_summary); } if fold_range.end > fold_range.start { @@ -438,32 +453,14 @@ impl FoldMap { if sum.input.len < edit.new.end.0 { let text_summary = inlay_snapshot .text_summary_for_range(InlayOffset(sum.input.len)..edit.new.end); - new_transforms.push( - Transform { - summary: TransformSummary { - output: text_summary.clone(), - input: text_summary, - }, - placeholder: None, - }, - &(), - ); + push_isomorphic(&mut new_transforms, text_summary); } } new_transforms.append(cursor.suffix(&()), &()); if new_transforms.is_empty() { let text_summary = inlay_snapshot.text_summary(); - new_transforms.push( - Transform { - summary: TransformSummary { - output: text_summary.clone(), - input: text_summary, - }, - placeholder: None, - }, - &(), - ); + push_isomorphic(&mut new_transforms, text_summary); } drop(cursor); @@ -715,17 +712,25 @@ impl FoldSnapshot { highlights: Highlights<'a>, ) -> FoldChunks<'a> { let mut transform_cursor = self.transforms.cursor::<(FoldOffset, InlayOffset)>(); + transform_cursor.seek(&range.start, Bias::Right, &()); - let inlay_end = { - transform_cursor.seek(&range.end, Bias::Right, &()); - let overshoot = range.end.0 - transform_cursor.start().0 .0; + let inlay_start = { + let overshoot = range.start.0 - transform_cursor.start().0 .0; transform_cursor.start().1 + InlayOffset(overshoot) }; - let inlay_start = { - transform_cursor.seek(&range.start, Bias::Right, &()); - let overshoot = range.start.0 - transform_cursor.start().0 .0; + let transform_end = transform_cursor.end(&()); + + let inlay_end = if transform_cursor + .item() + .map_or(true, |transform| transform.is_fold()) + { + inlay_start + } else if range.end < transform_end.0 { + let overshoot = range.end.0 - transform_cursor.start().0 .0; transform_cursor.start().1 + InlayOffset(overshoot) + } else { + transform_end.1 }; FoldChunks { @@ -737,8 +742,8 @@ impl FoldSnapshot { ), inlay_chunk: None, inlay_offset: inlay_start, - output_offset: range.start.0, - max_output_offset: range.end.0, + output_offset: range.start, + max_output_offset: range.end, } } @@ -783,6 +788,32 @@ impl FoldSnapshot { } } +fn push_isomorphic(transforms: &mut SumTree, summary: TextSummary) { + let mut did_merge = false; + transforms.update_last( + |last| { + if !last.is_fold() { + last.summary.input += summary.clone(); + last.summary.output += summary.clone(); + did_merge = true; + } + }, + &(), + ); + if !did_merge { + transforms.push( + Transform { + summary: TransformSummary { + input: summary.clone(), + output: summary, + }, + placeholder: None, + }, + &(), + ) + } +} + fn intersecting_folds<'a, T>( inlay_snapshot: &'a InlaySnapshot, folds: &'a SumTree, @@ -1079,8 +1110,8 @@ pub struct FoldChunks<'a> { inlay_chunks: InlayChunks<'a>, inlay_chunk: Option<(InlayOffset, Chunk<'a>)>, inlay_offset: InlayOffset, - output_offset: usize, - max_output_offset: usize, + output_offset: FoldOffset, + max_output_offset: FoldOffset, } impl<'a> Iterator for FoldChunks<'a> { @@ -1098,7 +1129,6 @@ impl<'a> Iterator for FoldChunks<'a> { if let Some(placeholder) = transform.placeholder.as_ref() { self.inlay_chunk.take(); self.inlay_offset += InlayOffset(transform.summary.input.len); - self.inlay_chunks.seek(self.inlay_offset); while self.inlay_offset >= self.transform_cursor.end(&()).1 && self.transform_cursor.item().is_some() @@ -1106,7 +1136,7 @@ impl<'a> Iterator for FoldChunks<'a> { self.transform_cursor.next(&()); } - self.output_offset += placeholder.text.len(); + self.output_offset.0 += placeholder.text.len(); return Some(Chunk { text: placeholder.text, renderer: Some(placeholder.renderer.clone()), @@ -1114,6 +1144,23 @@ impl<'a> Iterator for FoldChunks<'a> { }); } + // When we reach a non-fold region, seek the underlying text + // chunk iterator to the next unfolded range. + if self.inlay_offset == self.transform_cursor.start().1 + && self.inlay_chunks.offset() != self.inlay_offset + { + let transform_start = self.transform_cursor.start(); + let transform_end = self.transform_cursor.end(&()); + let inlay_end = if self.max_output_offset < transform_end.0 { + let overshoot = self.max_output_offset.0 - transform_start.0 .0; + transform_start.1 + InlayOffset(overshoot) + } else { + transform_end.1 + }; + + self.inlay_chunks.seek(self.inlay_offset..inlay_end); + } + // Retrieve a chunk from the current location in the buffer. if self.inlay_chunk.is_none() { let chunk_offset = self.inlay_chunks.offset(); @@ -1136,7 +1183,7 @@ impl<'a> Iterator for FoldChunks<'a> { } self.inlay_offset = chunk_end; - self.output_offset += chunk.text.len(); + self.output_offset.0 += chunk.text.len(); return Some(chunk); } diff --git a/crates/editor/src/display_map/inlay_map.rs b/crates/editor/src/display_map/inlay_map.rs index 82565e56b6..79da2d8c0e 100644 --- a/crates/editor/src/display_map/inlay_map.rs +++ b/crates/editor/src/display_map/inlay_map.rs @@ -225,14 +225,16 @@ pub struct InlayChunks<'a> { } impl<'a> InlayChunks<'a> { - pub fn seek(&mut self, offset: InlayOffset) { - self.transforms.seek(&offset, Bias::Right, &()); + pub fn seek(&mut self, new_range: Range) { + self.transforms.seek(&new_range.start, Bias::Right, &()); - let buffer_offset = self.snapshot.to_buffer_offset(offset); - self.buffer_chunks.seek(buffer_offset); + let buffer_range = self.snapshot.to_buffer_offset(new_range.start) + ..self.snapshot.to_buffer_offset(new_range.end); + self.buffer_chunks.seek(buffer_range); self.inlay_chunks = None; self.buffer_chunk = None; - self.output_offset = offset; + self.output_offset = new_range.start; + self.max_output_offset = new_range.end; } pub fn offset(&self) -> InlayOffset { diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index ba563c26ea..6bd9157f20 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -3668,6 +3668,63 @@ fn test_duplicate_line(cx: &mut TestAppContext) { ); }); } +#[gpui::test] +async fn test_fold_perf(cx: &mut TestAppContext) { + use std::fmt::Write; + init_test(cx, |_| {}); + let mut view = EditorTestContext::new(cx).await; + let language_registry = view.language_registry(); + let language_name = Arc::from("Markdown"); + let md_language = Arc::new( + Language::new( + LanguageConfig { + name: Arc::clone(&language_name), + matcher: LanguageMatcher { + path_suffixes: vec!["md".to_string()], + ..Default::default() + }, + ..Default::default() + }, + Some(tree_sitter_md::language()), + ) + .with_highlights_query( + r#" + "#, + ) + .unwrap(), + ); + language_registry.add(md_language.clone()); + + let mut text = String::default(); + writeln!(&mut text, "start").unwrap(); + writeln!(&mut text, "```").unwrap(); + const LINE_COUNT: u32 = 10000; + for i in 0..LINE_COUNT { + writeln!(&mut text, "{i}").unwrap(); + } + + writeln!(&mut text, "```").unwrap(); + writeln!(&mut text, "end").unwrap(); + view.update_buffer(|buffer, cx| { + buffer.set_language(Some(md_language), cx); + }); + let t0 = Instant::now(); + _ = view.update_editor(|view, cx| { + eprintln!("Text length: {}", text.len()); + view.set_text(text, cx); + eprintln!(">>"); + view.fold_ranges( + vec![( + Point::new(1, 0)..Point::new(LINE_COUNT + 2, 3), + FoldPlaceholder::test(), + )], + false, + cx, + ); + }); + eprintln!("{:?}", t0.elapsed()); + eprintln!("<<"); +} #[gpui::test] fn test_move_line_up_down(cx: &mut TestAppContext) { diff --git a/crates/language/src/buffer.rs b/crates/language/src/buffer.rs index 45ec698147..d3f5e6a71d 100644 --- a/crates/language/src/buffer.rs +++ b/crates/language/src/buffer.rs @@ -449,6 +449,7 @@ struct BufferChunkHighlights<'a> { /// An iterator that yields chunks of a buffer's text, along with their /// syntax highlights and diagnostic status. pub struct BufferChunks<'a> { + buffer_snapshot: Option<&'a BufferSnapshot>, range: Range, chunks: text::Chunks<'a>, diagnostic_endpoints: Peekable>, @@ -2475,6 +2476,17 @@ impl BufferSnapshot { None } + fn get_highlights(&self, range: Range) -> (SyntaxMapCaptures, Vec) { + let captures = self.syntax.captures(range, &self.text, |grammar| { + grammar.highlights_query.as_ref() + }); + let highlight_maps = captures + .grammars() + .into_iter() + .map(|grammar| grammar.highlight_map()) + .collect(); + (captures, highlight_maps) + } /// Iterates over chunks of text in the given range of the buffer. Text is chunked /// in an arbitrary way due to being stored in a [`Rope`](text::Rope). The text is also /// returned in chunks where each chunk has a single syntax highlighting style and @@ -2483,36 +2495,11 @@ impl BufferSnapshot { let range = range.start.to_offset(self)..range.end.to_offset(self); let mut syntax = None; - let mut diagnostic_endpoints = Vec::new(); if language_aware { - let captures = self.syntax.captures(range.clone(), &self.text, |grammar| { - grammar.highlights_query.as_ref() - }); - let highlight_maps = captures - .grammars() - .into_iter() - .map(|grammar| grammar.highlight_map()) - .collect(); - syntax = Some((captures, highlight_maps)); - for entry in self.diagnostics_in_range::<_, usize>(range.clone(), false) { - diagnostic_endpoints.push(DiagnosticEndpoint { - offset: entry.range.start, - is_start: true, - severity: entry.diagnostic.severity, - is_unnecessary: entry.diagnostic.is_unnecessary, - }); - diagnostic_endpoints.push(DiagnosticEndpoint { - offset: entry.range.end, - is_start: false, - severity: entry.diagnostic.severity, - is_unnecessary: entry.diagnostic.is_unnecessary, - }); - } - diagnostic_endpoints - .sort_unstable_by_key(|endpoint| (endpoint.offset, !endpoint.is_start)); + syntax = Some(self.get_highlights(range.clone())); } - BufferChunks::new(self.text.as_rope(), range, syntax, diagnostic_endpoints) + BufferChunks::new(self.text.as_rope(), range, syntax, Some(self)) } /// Invokes the given callback for each line of text in the given range of the buffer. @@ -2936,7 +2923,7 @@ impl BufferSnapshot { } let mut offset = buffer_range.start; - chunks.seek(offset); + chunks.seek(buffer_range.clone()); for mut chunk in chunks.by_ref() { if chunk.text.len() > buffer_range.end - offset { chunk.text = &chunk.text[0..(buffer_range.end - offset)]; @@ -3731,7 +3718,7 @@ impl<'a> BufferChunks<'a> { text: &'a Rope, range: Range, syntax: Option<(SyntaxMapCaptures<'a>, Vec)>, - diagnostic_endpoints: Vec, + buffer_snapshot: Option<&'a BufferSnapshot>, ) -> Self { let mut highlights = None; if let Some((captures, highlight_maps)) = syntax { @@ -3743,11 +3730,12 @@ impl<'a> BufferChunks<'a> { }) } - let diagnostic_endpoints = diagnostic_endpoints.into_iter().peekable(); + let diagnostic_endpoints = Vec::new().into_iter().peekable(); let chunks = text.chunks_in_range(range.clone()); - BufferChunks { + let mut this = BufferChunks { range, + buffer_snapshot, chunks, diagnostic_endpoints, error_depth: 0, @@ -3756,30 +3744,72 @@ impl<'a> BufferChunks<'a> { hint_depth: 0, unnecessary_depth: 0, highlights, - } + }; + this.initialize_diagnostic_endpoints(); + this } /// Seeks to the given byte offset in the buffer. - pub fn seek(&mut self, offset: usize) { - self.range.start = offset; - self.chunks.seek(self.range.start); + pub fn seek(&mut self, range: Range) { + let old_range = std::mem::replace(&mut self.range, range.clone()); + self.chunks.set_range(self.range.clone()); if let Some(highlights) = self.highlights.as_mut() { - highlights - .stack - .retain(|(end_offset, _)| *end_offset > offset); - if let Some(capture) = &highlights.next_capture { - if offset >= capture.node.start_byte() { - let next_capture_end = capture.node.end_byte(); - if offset < next_capture_end { - highlights.stack.push(( - next_capture_end, - highlights.highlight_maps[capture.grammar_index].get(capture.index), - )); + if old_range.start >= self.range.start && old_range.end <= self.range.end { + // Reuse existing highlights stack, as the new range is a subrange of the old one. + highlights + .stack + .retain(|(end_offset, _)| *end_offset > range.start); + if let Some(capture) = &highlights.next_capture { + if range.start >= capture.node.start_byte() { + let next_capture_end = capture.node.end_byte(); + if range.start < next_capture_end { + highlights.stack.push(( + next_capture_end, + highlights.highlight_maps[capture.grammar_index].get(capture.index), + )); + } + highlights.next_capture.take(); } - highlights.next_capture.take(); } + } else if let Some(snapshot) = self.buffer_snapshot { + let (captures, highlight_maps) = snapshot.get_highlights(self.range.clone()); + *highlights = BufferChunkHighlights { + captures, + next_capture: None, + stack: Default::default(), + highlight_maps, + }; + } else { + // We cannot obtain new highlights for a language-aware buffer iterator, as we don't have a buffer snapshot. + // Seeking such BufferChunks is not supported. + debug_assert!(false, "Attempted to seek on a language-aware buffer iterator without associated buffer snapshot"); } + highlights.captures.set_byte_range(self.range.clone()); + self.initialize_diagnostic_endpoints(); + } + } + + fn initialize_diagnostic_endpoints(&mut self) { + if let Some(buffer) = self.buffer_snapshot { + let mut diagnostic_endpoints = Vec::new(); + for entry in buffer.diagnostics_in_range::<_, usize>(self.range.clone(), false) { + diagnostic_endpoints.push(DiagnosticEndpoint { + offset: entry.range.start, + is_start: true, + severity: entry.diagnostic.severity, + is_unnecessary: entry.diagnostic.is_unnecessary, + }); + diagnostic_endpoints.push(DiagnosticEndpoint { + offset: entry.range.end, + is_start: false, + severity: entry.diagnostic.severity, + is_unnecessary: entry.diagnostic.is_unnecessary, + }); + } + diagnostic_endpoints + .sort_unstable_by_key(|endpoint| (endpoint.offset, !endpoint.is_start)); + self.diagnostic_endpoints = diagnostic_endpoints.into_iter().peekable(); } } diff --git a/crates/language/src/language.rs b/crates/language/src/language.rs index 3543a880c1..e700f5e538 100644 --- a/crates/language/src/language.rs +++ b/crates/language/src/language.rs @@ -1341,7 +1341,7 @@ impl Language { }); let highlight_maps = vec![grammar.highlight_map()]; let mut offset = 0; - for chunk in BufferChunks::new(text, range, Some((captures, highlight_maps)), vec![]) { + for chunk in BufferChunks::new(text, range, Some((captures, highlight_maps)), None) { let end_offset = offset + chunk.text.len(); if let Some(highlight_id) = chunk.syntax_highlight_id { if !highlight_id.is_default() { diff --git a/crates/multi_buffer/src/multi_buffer.rs b/crates/multi_buffer/src/multi_buffer.rs index 7017862bb1..5d9c62fdb3 100644 --- a/crates/multi_buffer/src/multi_buffer.rs +++ b/crates/multi_buffer/src/multi_buffer.rs @@ -2425,7 +2425,7 @@ impl MultiBufferSnapshot { excerpt_chunks: None, language_aware, }; - chunks.seek(range.start); + chunks.seek(range); chunks } @@ -4164,10 +4164,19 @@ impl Excerpt { } } - fn seek_chunks(&self, excerpt_chunks: &mut ExcerptChunks, offset: usize) { + fn seek_chunks(&self, excerpt_chunks: &mut ExcerptChunks, range: Range) { let content_start = self.range.context.start.to_offset(&self.buffer); - let chunks_start = content_start + offset; - excerpt_chunks.content_chunks.seek(chunks_start); + let chunks_start = content_start + range.start; + let chunks_end = content_start + cmp::min(range.end, self.text_summary.len); + excerpt_chunks.content_chunks.seek(chunks_start..chunks_end); + excerpt_chunks.footer_height = if self.has_trailing_newline + && range.start <= self.text_summary.len + && range.end > self.text_summary.len + { + 1 + } else { + 0 + }; } fn bytes_in_range(&self, range: Range) -> ExcerptBytes { @@ -4504,9 +4513,9 @@ impl<'a> MultiBufferChunks<'a> { self.range.start } - pub fn seek(&mut self, offset: usize) { - self.range.start = offset; - self.excerpts.seek(&offset, Bias::Right, &()); + pub fn seek(&mut self, new_range: Range) { + self.range = new_range.clone(); + self.excerpts.seek(&new_range.start, Bias::Right, &()); if let Some(excerpt) = self.excerpts.item() { let excerpt_start = self.excerpts.start(); if let Some(excerpt_chunks) = self @@ -4514,7 +4523,10 @@ impl<'a> MultiBufferChunks<'a> { .as_mut() .filter(|chunks| excerpt.id == chunks.excerpt_id) { - excerpt.seek_chunks(excerpt_chunks, self.range.start - excerpt_start); + excerpt.seek_chunks( + excerpt_chunks, + self.range.start - excerpt_start..self.range.end - excerpt_start, + ); } else { self.excerpt_chunks = Some(excerpt.chunks_in_range( self.range.start - excerpt_start..self.range.end - excerpt_start, diff --git a/crates/rope/src/rope.rs b/crates/rope/src/rope.rs index 8c76241fb7..a675081be4 100644 --- a/crates/rope/src/rope.rs +++ b/crates/rope/src/rope.rs @@ -615,6 +615,11 @@ impl<'a> Chunks<'a> { self.offset = offset; } + pub fn set_range(&mut self, range: Range) { + self.range = range.clone(); + self.seek(range.start); + } + /// Moves this cursor to the start of the next line in the rope. /// /// This method advances the cursor to the beginning of the next line.