diff --git a/crates/benchmarks/Cargo.toml b/crates/benchmarks/Cargo.toml index 20f980a8811..5a62d0273d9 100644 --- a/crates/benchmarks/Cargo.toml +++ b/crates/benchmarks/Cargo.toml @@ -21,7 +21,7 @@ criterion.workspace = true editor = { workspace = true, features = ["test-support"] } futures.workspace = true gpui = { workspace = true, features = ["bench"] } -gpui_platform = { workspace = true, features = ["test-support"] } +gpui_platform = { workspace = true, features = ["test-support", "font-kit"] } itertools.workspace = true language = { workspace = true, features = ["test-support"] } language_model = { workspace = true, features = ["test-support"] } diff --git a/crates/benchmarks/benches/editor_render.rs b/crates/benchmarks/benches/editor_render.rs index 91885fa74a5..67433bdb225 100644 --- a/crates/benchmarks/benches/editor_render.rs +++ b/crates/benchmarks/benches/editor_render.rs @@ -1,19 +1,23 @@ -use criterion::{Bencher, BenchmarkId}; use editor::{ Editor, EditorMode, MultiBuffer, actions::{DeleteToPreviousWordStart, SelectAll, SplitSelectionIntoLines}, }; -use gpui::{AppContext as _, BenchAppContext, Focusable as _, TestAppContext, TestDispatcher}; +use gpui::{AppContext as _, BenchAppContext, Focusable as _}; use rand::{Rng as _, SeedableRng as _, rngs::StdRng}; use settings::SettingsStore; use util::RandomCharIter; use zed_actions::editor::{MoveDown, MoveUp}; -#[gpui::bench] -fn editor_input_with_1000_cursors(cx: &mut BenchAppContext) { +#[gpui::bench( + inputs = multi_cursor_line_counts(), + group = "Multi-cursor input", + input_name = "cursors", + sample_size = 10 +)] +fn editor_multi_cursor_input(line_count: &usize, cx: &mut BenchAppContext) { init_context(cx); - let text = String::from_iter(["line:\n"; 1000]); + let text = "line:\n".repeat(*line_count); let buffer = cx.update(|cx| MultiBuffer::build_simple(&text, cx)); let mut window = cx.add_empty_window(); @@ -60,15 +64,16 @@ fn editor_input_with_1000_cursors(cx: &mut BenchAppContext) { }); } -fn open_editor_with_one_long_line(bencher: &mut Bencher<'_>, args: &(String, TestAppContext)) { - let (text, cx) = args; - let mut cx = cx.clone(); +#[gpui::bench] +fn open_editor_with_one_long_line(cx: &mut BenchAppContext) { + init_context(cx); - bencher.iter(|| { - let buffer = cx.update(|cx| MultiBuffer::build_simple(text, cx)); + let text = String::from_iter(["char"; 1000]); + cx.bench_iter(move |cx| { + let buffer = cx.update(|cx| MultiBuffer::build_simple(&text, cx)); - let cx = cx.add_empty_window(); - cx.update(|window, cx| { + let mut window = cx.add_empty_window(); + window.update(|window, cx| { let editor = cx.new(|cx| { let mut editor = Editor::new(EditorMode::full(), buffer, None, window, cx); editor.set_style(editor::EditorStyle::default(), window, cx); @@ -129,36 +134,18 @@ fn init_context(cx: &mut BenchAppContext) { }); } -fn init_test_context(cx: &TestAppContext) { - cx.update(|cx| { - let store = SettingsStore::test(cx); - cx.set_global(store); - assets::Assets.load_test_fonts(cx); - theme_settings::init(theme::LoadThemes::JustBase, cx); - editor::init(cx); - }); -} - -fn criterion_benches(criterion: &mut criterion::Criterion) { - let dispatcher = TestDispatcher::new(1); - let cx = gpui::TestAppContext::build(dispatcher, None); - init_test_context(&cx); - - let text = String::from_iter(["char"; 1000]); - let input = (text, cx); - let mut group = criterion.benchmark_group("Build buffer with one long line"); - group.bench_with_input( - BenchmarkId::new("editor_with_one_long_line", "(String, TestAppContext )"), - &input, - open_editor_with_one_long_line, - ); - group.finish(); +fn multi_cursor_line_counts() -> Vec { + let mut line_counts = vec![1000, 10000]; + if std::env::var("ZED_BENCH_HUGE").is_ok() { + line_counts.push(100000); + } + line_counts } gpui::bench_group!( benches, - editor_input_with_1000_cursors, - editor_render, - criterion_benches + editor_multi_cursor_input, + open_editor_with_one_long_line, + editor_render ); gpui::bench_main!(benches); diff --git a/crates/editor/src/display_map.rs b/crates/editor/src/display_map.rs index 5ba59083cf9..522f58a25fa 100644 --- a/crates/editor/src/display_map.rs +++ b/crates/editor/src/display_map.rs @@ -1547,6 +1547,11 @@ impl DisplaySnapshot { &self.block_snapshot.wrap_snapshot.tab_snapshot.fold_snapshot } + #[inline(always)] + pub fn has_collapsed_content(&self) -> bool { + self.fold_snapshot().has_folds() || self.block_snapshot.has_replacement_blocks() + } + pub fn inlay_snapshot(&self) -> &InlaySnapshot { &self .block_snapshot diff --git a/crates/editor/src/display_map/block_map.rs b/crates/editor/src/display_map/block_map.rs index 0aa8b323da3..5093d56d34b 100644 --- a/crates/editor/src/display_map/block_map.rs +++ b/crates/editor/src/display_map/block_map.rs @@ -331,6 +331,8 @@ impl std::fmt::Display for BlockId { #[derive(Clone, Debug)] struct Transform { summary: TransformSummary, + /// When `block` is `None`, the transform is isomorphic and passes input + /// wrap rows through as normal text. block: Option, } @@ -510,6 +512,7 @@ struct TransformSummary { output_rows: BlockRow, longest_row: BlockRow, longest_row_chars: u32, + has_replacement_blocks: bool, } pub struct BlockChunks<'a> { @@ -1089,6 +1092,7 @@ impl BlockMap { output_rows: BlockRow(block.height()), longest_row: BlockRow(0), longest_row_chars: 0, + has_replacement_blocks: false, }; let rows_before_block; @@ -1599,6 +1603,7 @@ fn push_isomorphic(tree: &mut SumTree, rows: RowDelta, wrap_snapshot: output_rows: BlockRow(rows.0), longest_row: BlockRow(wrap_summary.longest_row), longest_row_chars: wrap_summary.longest_row_chars, + has_replacement_blocks: false, }; let mut merged = false; tree.update_last( @@ -2145,6 +2150,11 @@ impl BlockMapWriter<'_> { } impl BlockSnapshot { + #[inline(always)] + pub fn has_replacement_blocks(&self) -> bool { + self.transforms.summary().has_replacement_blocks + } + #[cfg(test)] #[ztracing::instrument(skip_all)] pub fn text(&self) -> String { @@ -2755,7 +2765,9 @@ impl sum_tree::Item for Transform { type Summary = TransformSummary; fn summary(&self, _cx: ()) -> Self::Summary { - self.summary.clone() + let mut summary = self.summary.clone(); + summary.has_replacement_blocks = self.block.as_ref().is_some_and(Block::is_replacement); + summary } } @@ -2771,6 +2783,7 @@ impl sum_tree::ContextLessSummary for TransformSummary { } self.input_rows += summary.input_rows; self.output_rows += summary.output_rows; + self.has_replacement_blocks |= summary.has_replacement_blocks; } } diff --git a/crates/editor/src/display_map/fold_map.rs b/crates/editor/src/display_map/fold_map.rs index 4c6c04b86cc..fa677f03c19 100644 --- a/crates/editor/src/display_map/fold_map.rs +++ b/crates/editor/src/display_map/fold_map.rs @@ -722,6 +722,11 @@ impl FoldSnapshot { self.folds.items(&self.inlay_snapshot.buffer).len() } + #[inline(always)] + pub fn has_folds(&self) -> bool { + !self.folds.is_empty() + } + #[ztracing::instrument(skip_all)] pub fn text_summary_for_range(&self, range: Range) -> MBTextSummary { let mut summary = MBTextSummary::default(); diff --git a/crates/editor/src/display_map/inlay_map.rs b/crates/editor/src/display_map/inlay_map.rs index 016417ceed5..b1672a94bd7 100644 --- a/crates/editor/src/display_map/inlay_map.rs +++ b/crates/editor/src/display_map/inlay_map.rs @@ -82,6 +82,12 @@ struct TransformSummary { output: MBTextSummary, } +impl TransformSummary { + fn has_inlays(&self) -> bool { + self.input.len != self.output.len + } +} + impl sum_tree::ContextLessSummary for TransformSummary { fn zero() -> Self { Default::default() @@ -593,6 +599,29 @@ impl InlayMap { snapshot.buffer = buffer_snapshot; (snapshot.clone(), Vec::new()) + } else if self.inlays.is_empty() && !snapshot.transforms.summary().has_inlays() { + // Fast path: without inlays, the InlayMap is a passthrough, so rebuild a single + // isomorphic transform and forward buffer edits as inlay edits verbatim. + let mut new_transforms = SumTree::default(); + push_isomorphic(&mut new_transforms, buffer_snapshot.text_summary()); + if new_transforms.is_empty() { + new_transforms.push(Transform::Isomorphic(Default::default()), ()); + } + + let mut inlay_edits = Patch::default(); + for buffer_edit in &buffer_edits { + inlay_edits.push(Edit { + old: InlayOffset(buffer_edit.old.start)..InlayOffset(buffer_edit.old.end), + new: InlayOffset(buffer_edit.new.start)..InlayOffset(buffer_edit.new.end), + }); + } + + snapshot.transforms = new_transforms; + snapshot.version += 1; + snapshot.buffer = buffer_snapshot; + snapshot.check_invariants(); + + (snapshot.clone(), inlay_edits.into_inner()) } else { let mut inlay_edits = Patch::default(); let mut new_transforms = SumTree::default(); diff --git a/crates/editor/src/display_map/wrap_map.rs b/crates/editor/src/display_map/wrap_map.rs index 1d495965afd..9d2e016781b 100644 --- a/crates/editor/src/display_map/wrap_map.rs +++ b/crates/editor/src/display_map/wrap_map.rs @@ -66,6 +66,12 @@ struct TransformSummary { output: TextSummary, } +impl TransformSummary { + fn has_wraps(&self) -> bool { + self.input.lines != self.output.lines + } +} + #[derive(Copy, Clone, Debug, Default, Eq, Ord, PartialOrd, PartialEq)] pub struct WrapPoint(pub Point); @@ -386,6 +392,12 @@ impl WrapSnapshot { let mut new_transforms; if tab_edits.is_empty() { new_transforms = self.transforms.clone(); + } else if !self.transforms.summary().has_wraps() + && !new_tab_snapshot.text_summary().lines.is_zero() + { + // Fast path: without existing wraps, interpolation is a passthrough over the new tab snapshot. + new_transforms = SumTree::default(); + new_transforms.push(Transform::isomorphic(new_tab_snapshot.text_summary()), ()); } else { let mut old_cursor = self.transforms.cursor::(()); diff --git a/crates/editor/src/element.rs b/crates/editor/src/element.rs index 565852215a1..459442342c8 100644 --- a/crates/editor/src/element.rs +++ b/crates/editor/src/element.rs @@ -8256,30 +8256,31 @@ impl Element for EditorElement { .editor_with_selections(cx) .map(|editor| { editor.update(cx, |editor, cx| { - let all_selections = - editor.selections.all::(&snapshot.display_snapshot); - let all_anchor_selections = - editor.selections.all_anchors(&snapshot.display_snapshot); - let selected_buffer_ids = - if editor.buffer_kind(cx) == ItemBufferKind::Singleton { - Vec::new() - } else { - let mut selected_buffer_ids = - Vec::with_capacity(all_selections.len()); + let is_singleton = + editor.buffer_kind(cx) == ItemBufferKind::Singleton; - for selection in all_selections { - for buffer_id in snapshot - .buffer_snapshot() - .buffer_ids_for_range(selection.range()) - { - if selected_buffer_ids.last() != Some(&buffer_id) { - selected_buffer_ids.push(buffer_id); - } + // Singleton buffers only need the newest selection anchor here. + let selected_buffer_ids = if is_singleton { + Vec::new() + } else { + let all_selections = + editor.selections.all::(&snapshot.display_snapshot); + let mut selected_buffer_ids = + Vec::with_capacity(all_selections.len()); + + for selection in all_selections { + for buffer_id in snapshot + .buffer_snapshot() + .buffer_ids_for_range(selection.range()) + { + if selected_buffer_ids.last() != Some(&buffer_id) { + selected_buffer_ids.push(buffer_id); } } + } - selected_buffer_ids - }; + selected_buffer_ids + }; let mut selections = editor.selections.disjoint_in_range( start_anchor..end_anchor, @@ -8288,28 +8289,45 @@ impl Element for EditorElement { selections .extend(editor.selections.pending(&snapshot.display_snapshot)); - let mut anchors_by_buffer: HashMap = - HashMap::default(); - for selection in all_anchor_selections.iter() { - let head = selection.head(); - if let Some((text_anchor, _)) = - snapshot.buffer_snapshot().anchor_to_buffer_anchor(head) - { + let latest_selection_anchors: HashMap = + if is_singleton { + let head = editor.selections.newest_anchor().head(); + snapshot + .buffer_snapshot() + .anchor_to_buffer_anchor(head) + .map(|(text_anchor, _)| (text_anchor.buffer_id, head)) + .into_iter() + .collect() + } else { + let all_anchor_selections = editor + .selections + .all_anchors(&snapshot.display_snapshot); + let mut anchors_by_buffer: HashMap< + BufferId, + (usize, Anchor), + > = HashMap::default(); + for selection in all_anchor_selections.iter() { + let head = selection.head(); + if let Some((text_anchor, _)) = snapshot + .buffer_snapshot() + .anchor_to_buffer_anchor(head) + { + anchors_by_buffer + .entry(text_anchor.buffer_id) + .and_modify(|(latest_id, latest_anchor)| { + if selection.id > *latest_id { + *latest_id = selection.id; + *latest_anchor = head; + } + }) + .or_insert((selection.id, head)); + } + } anchors_by_buffer - .entry(text_anchor.buffer_id) - .and_modify(|(latest_id, latest_anchor)| { - if selection.id > *latest_id { - *latest_id = selection.id; - *latest_anchor = head; - } - }) - .or_insert((selection.id, head)); - } - } - let latest_selection_anchors = anchors_by_buffer - .into_iter() - .map(|(buffer_id, (_, anchor))| (buffer_id, anchor)) - .collect(); + .into_iter() + .map(|(buffer_id, (_, anchor))| (buffer_id, anchor)) + .collect() + }; (selections, selected_buffer_ids, latest_selection_anchors) }) diff --git a/crates/editor/src/scroll/autoscroll.rs b/crates/editor/src/scroll/autoscroll.rs index 6bdb60a470e..96386757248 100644 --- a/crates/editor/src/scroll/autoscroll.rs +++ b/crates/editor/src/scroll/autoscroll.rs @@ -171,17 +171,16 @@ impl Editor { target_top = target_point.row().as_f64(); target_bottom = target_top + 1.; } else { - let selections = self.selections.all::(&display_map); - - target_point = selections - .first() - .unwrap() + // Autoscroll only needs the first, last, and newest selections. + target_point = self + .selections + .first::(&display_map) .head() .to_display_point(&display_map); target_top = target_point.row().as_f64(); - target_bottom = selections - .last() - .unwrap() + target_bottom = self + .selections + .last::(&display_map) .head() .to_display_point(&display_map) .row() @@ -195,10 +194,9 @@ impl Editor { ) || (matches!(autoscroll, Autoscroll::Strategy(AutoscrollStrategy::Fit, _)) && !selections_fit) { - target_point = selections - .iter() - .max_by_key(|s| s.id) - .unwrap() + target_point = self + .selections + .newest::(&display_map) .head() .to_display_point(&display_map); target_top = target_point.row().as_f64(); @@ -361,7 +359,18 @@ impl Editor { let scroll_width = ScrollOffset::from(scroll_width); let display_map = self.display_map.update(cx, |map, cx| map.snapshot(cx)); - let selections = self.selections.all::(&display_map); + // Horizontal autoscroll only needs selections whose head is visible. + let visible_end_row = DisplayRow(start_row.0 + layouts.len() as u32); + let visible_range = display_map + .buffer_snapshot() + .anchor_before(DisplayPoint::new(start_row, 0).to_offset(&display_map, Bias::Left)) + ..display_map.buffer_snapshot().anchor_after( + DisplayPoint::new(visible_end_row, 0).to_offset(&display_map, Bias::Right), + ); + let mut selections = self + .selections + .disjoint_in_range::(visible_range, &display_map); + selections.extend(self.selections.pending::(&display_map)); let mut scroll_position = self.scroll_manager.scroll_position(&display_map, cx); let mut target_left; diff --git a/crates/editor/src/selections_collection.rs b/crates/editor/src/selections_collection.rs index 51dcca149ce..33ffdfca815 100644 --- a/crates/editor/src/selections_collection.rs +++ b/crates/editor/src/selections_collection.rs @@ -5,7 +5,7 @@ use std::{ }; use gpui::Pixels; -use itertools::Itertools as _; +use itertools::{Either, Itertools as _}; use language::{Bias, Point, PointUtf16, Selection, SelectionGoal}; use multi_buffer::{MultiBufferDimension, MultiBufferOffset}; use util::post_inc; @@ -123,11 +123,19 @@ impl SelectionsCollection { where D: MultiBufferDimension + Sub + AddAssign<::Output> + Ord, { - let disjoint_anchors = &self.disjoint; + self.all_iter(snapshot).collect() + } + + fn all_iter<'a, D>( + &'a self, + snapshot: &'a DisplaySnapshot, + ) -> impl 'a + Iterator> + where + D: 'a + MultiBufferDimension + Sub + AddAssign<::Output> + Ord, + { let mut disjoint = - resolve_selections_wrapping_blocks::(disjoint_anchors.iter(), &snapshot) - .peekable(); - let mut pending_opt = self.pending::(&snapshot); + resolve_selections_wrapping_blocks::(self.disjoint.iter(), snapshot).peekable(); + let mut pending_opt = self.pending::(snapshot); iter::from_fn(move || { if let Some(pending) = pending_opt.as_mut() { while let Some(next_selection) = disjoint.peek() { @@ -157,7 +165,6 @@ impl SelectionsCollection { disjoint.next() } }) - .collect() } /// Returns all of the selections, adjusted to take into account the selection line_mode @@ -317,14 +324,30 @@ impl SelectionsCollection { where D: MultiBufferDimension + Sub + AddAssign<::Output> + Ord, { - self.all(snapshot).first().unwrap().clone() + // Without a pending selection, the first disjoint selection is also the first resolved + // selection, so avoid resolving every selection. + if self.pending.is_none() + && let Some(first) = self.disjoint.first() + { + return resolve_selections_wrapping_blocks([first], snapshot) + .next() + .unwrap(); + } + self.all_iter(snapshot).next().unwrap() } pub fn last(&self, snapshot: &DisplaySnapshot) -> Selection where D: MultiBufferDimension + Sub + AddAssign<::Output> + Ord, { - self.all(snapshot).last().unwrap().clone() + if self.pending.is_none() + && let Some(last) = self.disjoint.last() + { + return resolve_selections_wrapping_blocks([last], snapshot) + .next() + .unwrap(); + } + self.all_iter(snapshot).last().unwrap() } /// Returns a list of (potentially backwards!) ranges representing the selections. @@ -1172,6 +1195,51 @@ pub(crate) fn resolve_selections_wrapping_blocks<'a, D, I>( selections: I, map: &'a DisplaySnapshot, ) -> impl 'a + Iterator> +where + D: MultiBufferDimension + Sub + AddAssign<::Output> + Ord, + I: 'a + IntoIterator>, +{ + // Without collapsed content, coalescing in buffer point space is equivalent to coalescing in + // display point space, so skip the per-selection display-coordinate round trip. + if !map.has_collapsed_content() { + Either::Left(resolve_selections_without_display_round_trip( + selections, map, + )) + } else { + Either::Right(resolve_selections_via_display_round_trip(selections, map)) + } +} + +fn resolve_selections_without_display_round_trip<'a, D, I>( + selections: I, + map: &'a DisplaySnapshot, +) -> impl 'a + Iterator> +where + D: MultiBufferDimension + Sub + AddAssign<::Output> + Ord, + I: 'a + IntoIterator>, +{ + let (to_convert, selections) = + coalesce_selections(resolve_selections_point(selections, map)).tee(); + let mut converted_endpoints = map + .buffer_snapshot() + .dimensions_from_points::(to_convert.flat_map(|s| [s.start, s.end])); + selections.map(move |s| { + let start = converted_endpoints.next().unwrap(); + let end = converted_endpoints.next().unwrap(); + Selection { + id: s.id, + start, + end, + reversed: s.reversed, + goal: s.goal, + } + }) +} + +fn resolve_selections_via_display_round_trip<'a, D, I>( + selections: I, + map: &'a DisplaySnapshot, +) -> impl 'a + Iterator> where D: MultiBufferDimension + Sub + AddAssign<::Output> + Ord, I: 'a + IntoIterator>, @@ -1268,3 +1336,135 @@ fn should_merge(a_start: T, a_end: T, b_start: T, b_end: T, sorte is_overlapping || same_start || cursor_at_boundary } + +#[cfg(test)] +mod tests { + use super::*; + use crate::{ + MultiBuffer, + display_map::{BlockPlacement, BlockProperties, BlockStyle, DisplayMap, FoldPlaceholder}, + test::test_font, + }; + use gpui::{AppContext as _, IntoElement as _, div, px}; + use project::project_settings::DiagnosticSeverity; + use rand::{Rng as _, rngs::StdRng}; + use settings::SettingsStore; + use std::sync::Arc; + + #[gpui::test(iterations = 20)] + fn fast_and_slow_selection_resolution_match_without_collapsed_content( + cx: &mut gpui::TestAppContext, + mut rng: StdRng, + ) { + cx.update(|cx| { + let settings = SettingsStore::test(cx); + cx.set_global(settings); + crate::init(cx); + }); + + let text = random_text(&mut rng); + let buffer = cx.update(|cx| MultiBuffer::build_simple(&text, cx)); + let display_map = cx.new(|cx| { + DisplayMap::new( + buffer.clone(), + test_font(), + px(14.), + Some(px(rng.random_range(80.0..=180.0))), + 1, + 1, + FoldPlaceholder::test(), + DiagnosticSeverity::Warning, + cx, + ) + }); + + let snapshot = display_map.update(cx, |map, cx| { + let buffer_snapshot = buffer.read(cx).snapshot(cx); + map.insert_blocks( + [BlockProperties { + placement: BlockPlacement::Above( + buffer_snapshot.anchor_after(MultiBufferOffset(text.len() / 2)), + ), + height: Some(2), + style: BlockStyle::Fixed, + render: Arc::new(|_| div().into_any_element()), + priority: 0, + }], + cx, + ); + map.snapshot(cx) + }); + assert!(!snapshot.has_collapsed_content()); + + let selections = random_anchor_selections(&snapshot, text.len(), &mut rng); + + let fast_points = + resolve_selections_without_display_round_trip::(selections.iter(), &snapshot) + .collect::>(); + let slow_points = + resolve_selections_via_display_round_trip::(selections.iter(), &snapshot) + .collect::>(); + assert_eq!(fast_points, slow_points); + + let fast_offsets = resolve_selections_without_display_round_trip::( + selections.iter(), + &snapshot, + ) + .collect::>(); + let slow_offsets = resolve_selections_via_display_round_trip::( + selections.iter(), + &snapshot, + ) + .collect::>(); + assert_eq!(fast_offsets, slow_offsets); + } + + fn random_text(rng: &mut StdRng) -> String { + let mut text = String::new(); + for line in 0..rng.random_range(8..32) { + if line > 0 { + text.push('\n'); + } + for _ in 0..rng.random_range(8..48) { + let ch = match rng.random_range(0..12) { + 0 => '\t', + 1 => ' ', + _ => rng.random_range(b'a'..=b'z') as char, + }; + text.push(ch); + } + } + text + } + + fn random_anchor_selections( + snapshot: &DisplaySnapshot, + text_len: usize, + rng: &mut StdRng, + ) -> Vec> { + let buffer = snapshot.buffer_snapshot(); + let mut selections = Vec::new(); + let mut offset = 0; + for id in 0..rng.random_range(1..32) { + if offset > text_len { + break; + } + + let start = rng.random_range(offset..=text_len); + let end = rng.random_range(start..=text_len); + selections.push(Selection { + id, + start: MultiBufferOffset(start), + end: MultiBufferOffset(end), + reversed: rng.random(), + goal: SelectionGoal::None, + }); + offset = end.saturating_add(rng.random_range(0..=4)); + } + + selections + .into_iter() + .map(|selection| selection_to_anchor_selection(selection, buffer)) + .collect() + } +} diff --git a/crates/gpui/src/app/bench_context.rs b/crates/gpui/src/app/bench_context.rs index aee68633f44..35d4144566f 100644 --- a/crates/gpui/src/app/bench_context.rs +++ b/crates/gpui/src/app/bench_context.rs @@ -11,56 +11,51 @@ use hdrhistogram::Histogram; use crate::{ AnyView, AnyWindowHandle, App, AppCell, AppContext, BackgroundExecutor, BenchDispatcher, - Bounds, Context, Empty, Entity, EntityId, Focusable, ForegroundExecutor, Global, - NoopTextSystem, Platform, PlatformHeadlessRenderer, Render, Reservation, Task, TestPlatform, + Bounds, Context, Empty, Entity, EntityId, Focusable, ForegroundExecutor, Global, Platform, + PlatformHeadlessRenderer, PlatformTextSystem, Render, Reservation, Task, TestPlatform, VisualContext, Window, WindowBounds, WindowHandle, WindowOptions, app::GpuiBorrow, profiler::{self, FrameTiming, FrameTimingCollector}, }; -/// Returns this thread's shared benchmark platform, creating it on first use. +/// Returns a benchmark platform backed by this thread's shared dispatcher. /// -/// The platform is a [`TestPlatform`] backed by a multithreaded -/// [`BenchDispatcher`], so background work runs with production concurrency in -/// real time. It is cached per thread and reused across benchmark invocations -/// so worker and timer threads persist for the whole process instead of being -/// recreated for every Criterion calibration pass. +/// The platform uses this thread's shared multithreaded [`BenchDispatcher`], so +/// background work runs with production concurrency in real time. The dispatcher +/// is cached per thread and reused across benchmark invocations so worker and +/// timer threads persist for the whole process instead of being recreated for +/// every Criterion calibration pass. /// -/// Text is shaped with [`NoopTextSystem`] (one glyph per character at fixed -/// advances). This keeps results deterministic across machines and font -/// installations while preserving the structure of downstream layout and paint -/// work; absolute timings exclude production text shaping (roughly 10% of draw -/// time for a full editor frame). Benchmarks that need real shaping can build -/// a [`TestPlatform`] with a platform text system and pass it to -/// [`BenchAppContext::new_with_platform_and_report`]. +/// Text is shaped with the provided platform text system. Benchmarks generated +/// by `#[gpui::bench]` use the current platform's text system, so text-heavy +/// benchmark measurements include production shaping and glyph rasterization. /// -/// `headless_renderer_factory` (only used on first call) supplies a renderer -/// for benchmark windows, e.g. `gpui_platform::current_headless_renderer`. -/// When present, scenes drawn by benchmarks are rasterized through the real -/// sprite atlas and submitted to the GPU on present, so quad/sprite -/// regressions show up in measurements. When `None`, presenting discards the -/// scene. Currently only macOS provides a headless renderer (Metal), so GPU -/// submission is excluded from benchmark measurements on other platforms. +/// `headless_renderer_factory` supplies a renderer for benchmark windows, e.g. +/// `gpui_platform::current_headless_renderer`. When present, scenes drawn by +/// benchmarks are rasterized through the real sprite atlas and submitted to +/// the GPU on present, so quad/sprite regressions show up in measurements. +/// When `None`, presenting discards the scene. Currently only macOS provides +/// a headless renderer (Metal), so GPU submission is excluded from benchmark +/// measurements on other platforms. pub fn bench_platform( headless_renderer_factory: Option Option>>>, + text_system: Arc, ) -> Rc { thread_local! { - static PLATFORM: OnceCell> = const { OnceCell::new() }; + static DISPATCHER: OnceCell> = const { OnceCell::new() }; } - PLATFORM.with(|cell| { - cell.get_or_init(|| { - let dispatcher = Arc::new(BenchDispatcher::new()); - let background_executor = BackgroundExecutor::new(dispatcher.clone()); - let foreground_executor = ForegroundExecutor::new(dispatcher); - TestPlatform::with_platform( - background_executor, - foreground_executor, - Arc::new(NoopTextSystem::new()), - headless_renderer_factory, - ) - }) - .clone() as Rc - }) + let dispatcher = DISPATCHER.with(|cell| { + cell.get_or_init(|| Arc::new(BenchDispatcher::new())) + .clone() + }); + let background_executor = BackgroundExecutor::new(dispatcher.clone()); + let foreground_executor = ForegroundExecutor::new(dispatcher); + TestPlatform::with_platform( + background_executor, + foreground_executor, + text_system, + headless_renderer_factory, + ) as Rc } /// Default target frame rate when a benchmark doesn't specify `fps = N`. @@ -442,9 +437,12 @@ impl<'a, 'measurement> BenchAppContext<'a, 'measurement> { /// Adds a window with an empty root view for benchmark setup. pub fn add_empty_window(&mut self) -> BenchWindowContext<'a, 'measurement> { + let bounds = { + let app = self.app.borrow(); + Bounds::maximized(None, &app) + }; let window = { let mut app = self.app.borrow_mut(); - let bounds = Bounds::maximized(None, &app); let window: AnyWindowHandle = app .open_window( WindowOptions { @@ -481,20 +479,34 @@ impl<'a, 'measurement> BenchAppContext<'a, 'measurement> { /// Runs GPUI benchmark teardown. /// - /// Forgets any timers still armed on the shared dispatcher so they can't - /// fire during a later benchmark; assumes no other `BenchAppContext` is - /// live on this thread. + /// Cancels any timers still armed on the shared dispatcher and drains the + /// work that cancellation unblocks so they can't fire during a later + /// benchmark; assumes no other `BenchAppContext` is live on this thread. pub fn teardown(mut self) { self.run_until_idle(); self.update(|cx| { cx.quit(); }); self.run_until_idle(); - self.background_executor - .dispatcher() + + let dispatcher = self.background_executor.dispatcher(); + let dispatcher = dispatcher .as_bench() - .expect("validated in BenchAppContext::build") - .forget_pending_timers(); + .expect("validated in BenchAppContext::build"); + + drop(self.app); + drop(self.foreground_executor); + + for _ in 0..100 { + if dispatcher.cancel_pending_timers() == 0 { + return; + } + dispatcher.run_until_idle(); + } + panic!( + "benchmark teardown kept scheduling timers: {}", + dispatcher.debug_state() + ); } } diff --git a/crates/gpui/src/platform/bench_dispatcher.rs b/crates/gpui/src/platform/bench_dispatcher.rs index 80aff1c3895..91e4a27c82f 100644 --- a/crates/gpui/src/platform/bench_dispatcher.rs +++ b/crates/gpui/src/platform/bench_dispatcher.rs @@ -258,16 +258,34 @@ impl BenchDispatcher { } } - /// Forgets all pending timers so timers armed by one benchmark can't fire + /// Cancels all pending timers so timers armed by one benchmark can't fire /// during a later benchmark sharing this process-lifetime dispatcher. /// - /// The runnables are leaked rather than dropped, since dropping one wakes - /// the awaiting task as if the timer had fired. - pub fn forget_pending_timers(&self) { - let mut state = self.timers.state.lock(); - for entry in state.heap.drain() { - std::mem::forget(entry.runnable); - } + /// Dropping a timer runnable drops its completion sender, waking the task + /// awaiting the timer. Call [`Self::run_until_idle`] after this method to + /// drain any work that cancellation unblocks. + pub fn cancel_pending_timers(&self) -> usize { + let timers = { + let mut state = self.timers.state.lock(); + let timers: Vec<_> = state.heap.drain().collect(); + self.timers.condvar.notify_all(); + timers + }; + let canceled = timers.len(); + drop(timers); + canceled + } + + /// Describes the dispatcher's idle-tracking state, for diagnosing + /// benchmarks that fail to reach quiescence. + pub fn debug_state(&self) -> String { + let inflight = *self.idle.inflight.lock(); + let timers = self.timers.state.lock().heap.len(); + let main_queue_has_work = self.main_queue_has_work(); + format!( + "BenchDispatcher {{ inflight: {inflight}, pending_timers: {timers}, \ + main_queue_has_work: {main_queue_has_work} }}" + ) } fn has_due_timer(&self) -> bool { @@ -415,12 +433,12 @@ mod tests { } #[test] - fn forget_pending_timers_prevents_stale_timers_from_firing() { + fn cancel_pending_timers_wakes_waiters_without_waiting_for_deadline() { let dispatcher = Arc::new(BenchDispatcher::new()); let background = BackgroundExecutor::new(dispatcher.clone()); let fired = Arc::new(AtomicBool::new(false)); - let timer = background.timer(Duration::from_millis(250)); + let timer = background.timer(Duration::from_secs(10)); background .spawn({ let fired = fired.clone(); @@ -432,10 +450,10 @@ mod tests { .detach(); dispatcher.run_until_idle(); - dispatcher.forget_pending_timers(); - - thread::sleep(Duration::from_millis(400)); + assert_eq!(dispatcher.cancel_pending_timers(), 1); dispatcher.run_until_idle(); - assert!(!fired.load(Ordering::SeqCst)); + + assert!(fired.load(Ordering::SeqCst)); + assert_eq!(dispatcher.cancel_pending_timers(), 0); } } diff --git a/crates/gpui_macros/src/bench.rs b/crates/gpui_macros/src/bench.rs index d5fad465e4b..8fea39ef0ec 100644 --- a/crates/gpui_macros/src/bench.rs +++ b/crates/gpui_macros/src/bench.rs @@ -1,9 +1,13 @@ use proc_macro::TokenStream; use quote::{format_ident, quote}; -use syn::{ItemFn, parse::Parser, spanned::Spanned}; +use syn::{Expr, ItemFn, LitStr, parse::Parser, spanned::Spanned}; pub fn bench(args: TokenStream, function: TokenStream) -> TokenStream { let mut fps: Option = None; + let mut inputs: Option = None; + let mut input_name: Option = None; + let mut group_name: Option = None; + let mut sample_size: Option = None; if !args.is_empty() { let parser = syn::meta::parser(|meta| { if meta.path.is_ident("fps") { @@ -14,8 +18,29 @@ pub fn bench(args: TokenStream, function: TokenStream) -> TokenStream { } fps = Some(value); Ok(()) + } else if meta.path.is_ident("inputs") { + inputs = Some(meta.value()?.parse()?); + Ok(()) + } else if meta.path.is_ident("input_name") { + input_name = Some(meta.value()?.parse()?); + Ok(()) + } else if meta.path.is_ident("group") { + group_name = Some(meta.value()?.parse()?); + Ok(()) + } else if meta.path.is_ident("sample_size") { + let value: syn::LitInt = meta.value()?.parse()?; + let value = value.base10_parse::()?; + if value == 0 { + return Err( + meta.error("#[gpui::bench] `sample_size` must be greater than zero") + ); + } + sample_size = Some(value); + Ok(()) } else { - Err(meta.error("#[gpui::bench] only accepts `fps = N`")) + Err(meta.error( + "#[gpui::bench] only accepts `fps = N`, `inputs = EXPR`, `input_name = \"...\"`, `group = \"...\"`, and `sample_size = N`", + )) } }); if let Err(error) = parser.parse(args) { @@ -46,18 +71,75 @@ pub fn bench(args: TokenStream, function: TokenStream) -> TokenStream { let inner_fn_name = format_ident!("__gpui_bench_{}", outer_fn_name); inner_fn.sig.ident = inner_fn_name.clone(); - TokenStream::from(quote! { - #inner_fn - - fn #outer_fn_name(criterion: &mut criterion::Criterion) { + let benchmark = if let Some(inputs) = inputs { + let input_name = match input_name { + Some(input_name) => quote! { #input_name }, + None => quote! { stringify!(#outer_fn_name) }, + }; + let group_name = match group_name { + Some(group_name) => quote! { #group_name }, + None => quote! { stringify!(#outer_fn_name) }, + }; + let sample_size = + sample_size.map(|sample_size| quote! { group.sample_size(#sample_size); }); + quote! { + let report = #report_expr; + let mut group = criterion.benchmark_group(#group_name); + #sample_size + for input in #inputs { + group.bench_with_input(criterion::BenchmarkId::new(#input_name, &input), &input, { + let report = report.clone(); + move |bencher, input| { + let mut cx = gpui::BenchAppContext::new_with_platform_and_report( + gpui::bench_platform( + Some(Box::new(|| { + gpui_platform::current_headless_renderer() + })), + gpui_platform::current_platform(true).text_system(), + ), + Some(stringify!(#outer_fn_name)), + bencher, + report.clone(), + ); + #inner_fn_name(input, &mut cx); + cx.teardown(); + } + }); + } + group.finish(); + report.print(Some(stringify!(#outer_fn_name))); + } + } else { + if let Some(input_name) = input_name { + return error_to_stream(syn::Error::new( + input_name.span(), + "#[gpui::bench] `input_name` requires `inputs`", + )); + } + if let Some(group_name) = group_name { + return error_to_stream(syn::Error::new( + group_name.span(), + "#[gpui::bench] `group` requires `inputs`", + )); + } + if sample_size.is_some() { + return error_to_stream(syn::Error::new( + proc_macro2::Span::call_site(), + "#[gpui::bench] `sample_size` requires `inputs`", + )); + } + quote! { let report = #report_expr; criterion.bench_function(stringify!(#outer_fn_name), { let report = report.clone(); move |bencher| { let mut cx = gpui::BenchAppContext::new_with_platform_and_report( - gpui::bench_platform(Some(Box::new(|| { - gpui_platform::current_headless_renderer() - }))), + gpui::bench_platform( + Some(Box::new(|| { + gpui_platform::current_headless_renderer() + })), + gpui_platform::current_platform(true).text_system(), + ), Some(stringify!(#outer_fn_name)), bencher, report.clone(), @@ -68,6 +150,14 @@ pub fn bench(args: TokenStream, function: TokenStream) -> TokenStream { }); report.print(Some(stringify!(#outer_fn_name))); } + }; + + TokenStream::from(quote! { + #inner_fn + + fn #outer_fn_name(criterion: &mut criterion::Criterion) { + #benchmark + } }) } diff --git a/crates/gpui_macros/src/gpui_macros.rs b/crates/gpui_macros/src/gpui_macros.rs index 41ad72c67ab..f3958bca568 100644 --- a/crates/gpui_macros/src/gpui_macros.rs +++ b/crates/gpui_macros/src/gpui_macros.rs @@ -192,6 +192,11 @@ pub fn test(args: TokenStream, function: TokenStream) -> TokenStream { /// `#[gpui::bench]` annotates a Criterion benchmark that runs with GPUI support. /// +/// Use `#[gpui::bench(inputs = some_iterable())]` on benchmarks that take an +/// additional input argument; the generated benchmark uses Criterion's +/// `bench_with_input`. `group`, `input_name`, and `sample_size` can customize +/// the generated input benchmark group. +/// /// The benchmark crate must add `criterion` and `gpui_platform` (with its /// `test-support` feature) to its dev-dependencies and enable gpui's `bench` /// feature, since the generated code references all three.