From ccce05d25cef856746103a25d909a22456e6e6c0 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Mon, 5 Jan 2026 15:18:09 +0100 Subject: [PATCH] language: Fix reparse timeout not actually working (#45347) We use `block_with_timeout` to give the reparse task a millisecond to complete, and if it takes longer we put the work off to the background. The reason for this is that we want tree-sitter based features to feel snappy. The reparse task is non-cooperative though, it has no yield points, giving us no place to actually check for our timeout, meaning we will always drive it to completion and block for the entire duration. This kicks out the `block_with_timeout` in favor of using the treesitter progress callback to handle timeouts instead. Release Notes: - Improved responsiveness with very language language files on edits --- crates/editor/src/editor_tests.rs | 2 +- crates/gpui/src/platform/windows/platform.rs | 3 - crates/language/src/buffer.rs | 124 ++++++++----------- crates/language/src/buffer_tests.rs | 8 +- crates/language/src/syntax_map.rs | 96 ++++++++++++-- crates/languages/src/typescript.rs | 1 + 6 files changed, 143 insertions(+), 91 deletions(-) diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index 73487bc43a776a22141eb77308df8f22a783fb30..4f60989cd47e3fd070aa2d2e309ac7e0f162b9c7 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -18233,7 +18233,7 @@ async fn test_on_type_formatting_is_applied_after_autoindent(cx: &mut TestAppCon cx.update_buffer(|buffer, _| { // This causes autoindent to be async. - buffer.set_sync_parse_timeout(Duration::ZERO) + buffer.set_sync_parse_timeout(None) }); cx.set_state("fn c() {\n d()ˇ\n}\n"); diff --git a/crates/gpui/src/platform/windows/platform.rs b/crates/gpui/src/platform/windows/platform.rs index 75085d0b4ec3cae5a156e1d9029e5d79353477e0..76643d1896cb1d3568883cb881541ba42a7fef65 100644 --- a/crates/gpui/src/platform/windows/platform.rs +++ b/crates/gpui/src/platform/windows/platform.rs @@ -838,9 +838,6 @@ impl WindowsPlatformInner { let peek_msg = |msg: &mut _, msg_kind| unsafe { PeekMessageW(msg, None, 0, 0, PM_REMOVE | msg_kind).as_bool() }; - if peek_msg(&mut msg, PM_QS_PAINT) { - process_message(&msg); - } while peek_msg(&mut msg, PM_QS_INPUT) { process_message(&msg); } diff --git a/crates/language/src/buffer.rs b/crates/language/src/buffer.rs index 800bb92fc079f33c147af730d27914f252235cf6..e63e83193912350341fd71018b4eb34e1d2dec24 100644 --- a/crates/language/src/buffer.rs +++ b/crates/language/src/buffer.rs @@ -121,7 +121,7 @@ pub struct Buffer { autoindent_requests: Vec>, wait_for_autoindent_txs: Vec>, pending_autoindent: Option>, - sync_parse_timeout: Duration, + sync_parse_timeout: Option, syntax_map: Mutex, reparse: Option>, parse_status: (watch::Sender, watch::Receiver), @@ -1124,7 +1124,7 @@ impl Buffer { syntax_map, reparse: None, non_text_state_update_count: 0, - sync_parse_timeout: Duration::from_millis(1), + sync_parse_timeout: Some(Duration::from_millis(1)), parse_status: watch::channel(ParseStatus::Idle), autoindent_requests: Default::default(), wait_for_autoindent_txs: Default::default(), @@ -1704,7 +1704,7 @@ impl Buffer { } #[cfg(any(test, feature = "test-support"))] - pub fn set_sync_parse_timeout(&mut self, timeout: Duration) { + pub fn set_sync_parse_timeout(&mut self, timeout: Option) { self.sync_parse_timeout = timeout; } @@ -1763,6 +1763,20 @@ impl Buffer { let mut syntax_snapshot = syntax_map.snapshot(); drop(syntax_map); + self.parse_status.0.send(ParseStatus::Parsing).unwrap(); + if may_block && let Some(sync_parse_timeout) = self.sync_parse_timeout { + if let Ok(()) = syntax_snapshot.reparse_with_timeout( + &text, + language_registry.clone(), + language.clone(), + sync_parse_timeout, + ) { + self.did_finish_parsing(syntax_snapshot, Duration::from_millis(300), cx); + self.reparse = None; + return; + } + } + let parse_task = cx.background_spawn({ let language = language.clone(); let language_registry = language_registry.clone(); @@ -1772,79 +1786,43 @@ impl Buffer { } }); - self.parse_status.0.send(ParseStatus::Parsing).unwrap(); - if may_block { - match cx - .background_executor() - .block_with_timeout(self.sync_parse_timeout, parse_task) - { - Ok(new_syntax_snapshot) => { - self.did_finish_parsing(new_syntax_snapshot, cx); - self.reparse = None; - } - Err(parse_task) => { - self.reparse = Some(cx.spawn(async move |this, cx| { - let new_syntax_map = cx.background_spawn(parse_task).await; - this.update(cx, move |this, cx| { - let grammar_changed = || { - this.language.as_ref().is_none_or(|current_language| { - !Arc::ptr_eq(&language, current_language) - }) - }; - let language_registry_changed = || { - new_syntax_map.contains_unknown_injections() - && language_registry.is_some_and(|registry| { - registry.version() - != new_syntax_map.language_registry_version() - }) - }; - let parse_again = this.version.changed_since(&parsed_version) - || language_registry_changed() - || grammar_changed(); - this.did_finish_parsing(new_syntax_map, cx); - this.reparse = None; - if parse_again { - this.reparse(cx, false); - } + self.reparse = Some(cx.spawn(async move |this, cx| { + let new_syntax_map = parse_task.await; + this.update(cx, move |this, cx| { + let grammar_changed = || { + this.language + .as_ref() + .is_none_or(|current_language| !Arc::ptr_eq(&language, current_language)) + }; + let language_registry_changed = || { + new_syntax_map.contains_unknown_injections() + && language_registry.is_some_and(|registry| { + registry.version() != new_syntax_map.language_registry_version() }) - .ok(); - })); + }; + let parse_again = this.version.changed_since(&parsed_version) + || language_registry_changed() + || grammar_changed(); + this.did_finish_parsing(new_syntax_map, Duration::ZERO, cx); + this.reparse = None; + if parse_again { + this.reparse(cx, false); } - } - } else { - self.reparse = Some(cx.spawn(async move |this, cx| { - let new_syntax_map = cx.background_spawn(parse_task).await; - this.update(cx, move |this, cx| { - let grammar_changed = || { - this.language.as_ref().is_none_or(|current_language| { - !Arc::ptr_eq(&language, current_language) - }) - }; - let language_registry_changed = || { - new_syntax_map.contains_unknown_injections() - && language_registry.is_some_and(|registry| { - registry.version() != new_syntax_map.language_registry_version() - }) - }; - let parse_again = this.version.changed_since(&parsed_version) - || language_registry_changed() - || grammar_changed(); - this.did_finish_parsing(new_syntax_map, cx); - this.reparse = None; - if parse_again { - this.reparse(cx, false); - } - }) - .ok(); - })); - } + }) + .ok(); + })); } - fn did_finish_parsing(&mut self, syntax_snapshot: SyntaxSnapshot, cx: &mut Context) { - self.was_changed(); + fn did_finish_parsing( + &mut self, + syntax_snapshot: SyntaxSnapshot, + block_budget: Duration, + cx: &mut Context, + ) { self.non_text_state_update_count += 1; self.syntax_map.lock().did_parse(syntax_snapshot); - self.request_autoindent(cx); + self.was_changed(); + self.request_autoindent(cx, block_budget); self.parse_status.0.send(ParseStatus::Idle).unwrap(); self.invalidate_tree_sitter_data(self.text.snapshot()); cx.emit(BufferEvent::Reparsed); @@ -1902,12 +1880,12 @@ impl Buffer { } } - fn request_autoindent(&mut self, cx: &mut Context) { + fn request_autoindent(&mut self, cx: &mut Context, block_budget: Duration) { if let Some(indent_sizes) = self.compute_autoindents() { let indent_sizes = cx.background_spawn(indent_sizes); match cx .background_executor() - .block_with_timeout(Duration::from_micros(500), indent_sizes) + .block_with_timeout(block_budget, indent_sizes) { Ok(indent_sizes) => self.apply_autoindents(indent_sizes, cx), Err(indent_sizes) => { @@ -2809,7 +2787,7 @@ impl Buffer { is_block_mode: false, ignore_empty_lines: true, })); - self.request_autoindent(cx); + self.request_autoindent(cx, Duration::from_micros(300)); } // Inserts newlines at the given position to create an empty line, returning the start of the new line. diff --git a/crates/language/src/buffer_tests.rs b/crates/language/src/buffer_tests.rs index 2c2d93c8239f0f3fcb1de0956de2d3400f13e96b..aef42c9f576a4bee52854e173da5510e9e182749 100644 --- a/crates/language/src/buffer_tests.rs +++ b/crates/language/src/buffer_tests.rs @@ -622,9 +622,7 @@ async fn test_reparse(cx: &mut gpui::TestAppContext) { ) ); - buffer.update(cx, |buffer, _| { - buffer.set_sync_parse_timeout(Duration::ZERO) - }); + buffer.update(cx, |buffer, _| buffer.set_sync_parse_timeout(None)); // Perform some edits (add parameter and variable reference) // Parsing doesn't begin until the transaction is complete @@ -736,7 +734,7 @@ async fn test_reparse(cx: &mut gpui::TestAppContext) { async fn test_resetting_language(cx: &mut gpui::TestAppContext) { let buffer = cx.new(|cx| { let mut buffer = Buffer::local("{}", cx).with_language(rust_lang(), cx); - buffer.set_sync_parse_timeout(Duration::ZERO); + buffer.set_sync_parse_timeout(None); buffer }); @@ -2359,7 +2357,7 @@ async fn test_async_autoindents_preserve_preview(cx: &mut TestAppContext) { let mut buffer = Buffer::local(text, cx).with_language(rust_lang(), cx); // This causes autoindent to be async. - buffer.set_sync_parse_timeout(Duration::ZERO); + buffer.set_sync_parse_timeout(None); buffer.edit([(8..8, "\n\n")], Some(AutoindentMode::EachLine), cx); buffer.refresh_preview(); diff --git a/crates/language/src/syntax_map.rs b/crates/language/src/syntax_map.rs index db4ab4f459c35a98752bef1eb5be558084b5c906..07dacd5216fc0f437f588f34b11e2a2acf715502 100644 --- a/crates/language/src/syntax_map.rs +++ b/crates/language/src/syntax_map.rs @@ -4,7 +4,6 @@ mod syntax_map_tests; use crate::{ Grammar, InjectionConfig, Language, LanguageId, LanguageRegistry, QUERY_CURSORS, with_parser, }; -use anyhow::Context as _; use collections::HashMap; use futures::FutureExt; use gpui::SharedString; @@ -13,8 +12,9 @@ use std::{ cmp::{self, Ordering, Reverse}, collections::BinaryHeap, fmt, iter, - ops::{Deref, DerefMut, Range}, + ops::{ControlFlow, Deref, DerefMut, Range}, sync::Arc, + time::{Duration, Instant}, }; use streaming_iterator::StreamingIterator; use sum_tree::{Bias, Dimensions, SeekTarget, SumTree}; @@ -421,11 +421,38 @@ impl SyntaxSnapshot { registry: Option>, root_language: Arc, ) { + self.reparse_(text, registry, root_language, None).ok(); + } + + pub fn reparse_with_timeout( + &mut self, + text: &BufferSnapshot, + registry: Option>, + root_language: Arc, + budget: Duration, + ) -> Result<(), ParseTimeout> { + self.reparse_(text, registry, root_language, Some(budget)) + } + + fn reparse_( + &mut self, + text: &BufferSnapshot, + registry: Option>, + root_language: Arc, + mut budget: Option, + ) -> Result<(), ParseTimeout> { + let budget = &mut budget; let edit_ranges = text .edits_since::(&self.parsed_version) .map(|edit| edit.new) .collect::>(); - self.reparse_with_ranges(text, root_language.clone(), edit_ranges, registry.as_ref()); + self.reparse_with_ranges( + text, + root_language.clone(), + edit_ranges, + registry.as_ref(), + budget, + )?; if let Some(registry) = registry && registry.version() != self.language_registry_version @@ -460,12 +487,14 @@ impl SyntaxSnapshot { root_language, resolved_injection_ranges, Some(®istry), - ); + budget, + )?; } self.language_registry_version = registry.version(); } self.update_count += 1; + Ok(()) } fn reparse_with_ranges( @@ -474,7 +503,8 @@ impl SyntaxSnapshot { root_language: Arc, invalidated_ranges: Vec>, registry: Option<&Arc>, - ) { + budget: &mut Option, + ) -> Result<(), ParseTimeout> { log::trace!( "reparse. invalidated ranges:{:?}", LogOffsetRanges(&invalidated_ranges, text), @@ -670,11 +700,15 @@ impl SyntaxSnapshot { step_start_byte, &included_ranges, Some(old_tree.clone()), + budget, ); match result { Ok(t) => tree = t, + Err(e) if e.downcast_ref::().is_some() => { + return Err(ParseTimeout); + } Err(e) => { - log::error!("error parsing text: {:?}", e); + log::error!("error parsing text: {e:?}"); continue; } }; @@ -723,11 +757,15 @@ impl SyntaxSnapshot { step_start_byte, &included_ranges, None, + budget, ); match result { Ok(t) => tree = t, + Err(e) if e.downcast_ref::().is_some() => { + return Err(ParseTimeout); + } Err(e) => { - log::error!("error parsing text: {:?}", e); + log::error!("error parsing text: {e:?}"); continue; } }; @@ -805,6 +843,7 @@ impl SyntaxSnapshot { self.parsed_version = text.version.clone(); #[cfg(debug_assertions)] self.check_invariants(text); + Ok(()) } #[cfg(debug_assertions)] @@ -1372,14 +1411,41 @@ fn join_ranges( result } +#[derive(Copy, Clone, Debug)] +pub struct ParseTimeout; + +impl std::error::Error for ParseTimeout {} + +impl std::fmt::Display for ParseTimeout { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "parse timeout") + } +} + fn parse_text( grammar: &Grammar, text: &Rope, start_byte: usize, ranges: &[tree_sitter::Range], old_tree: Option, + parse_budget: &mut Option, ) -> anyhow::Result { with_parser(|parser| { + let mut timed_out = false; + let now = Instant::now(); + let mut progress_callback = parse_budget.map(|budget| { + let timed_out = &mut timed_out; + move |_: &_| { + let elapsed = now.elapsed(); + if elapsed > budget { + *timed_out = true; + ControlFlow::Break(()) + } else { + ControlFlow::Continue(()) + } + } + }); + let mut chunks = text.chunks_in_range(start_byte..text.len()); parser.set_included_ranges(ranges)?; parser.set_language(&grammar.ts_language)?; @@ -1390,9 +1456,21 @@ fn parse_text( chunks.next().unwrap_or("").as_bytes() }, old_tree.as_ref(), - None, + progress_callback + .as_mut() + .map(|progress_callback| tree_sitter::ParseOptions { + progress_callback: Some(progress_callback), + }), ) - .context("failed to parse") + .inspect(|_| { + if let Some(parse_budget) = parse_budget { + *parse_budget = parse_budget.saturating_sub(now.elapsed()); + } + }) + .ok_or_else(|| match timed_out { + true => anyhow::anyhow!(ParseTimeout), + false => anyhow::anyhow!("parsing failed"), + }) }) } diff --git a/crates/languages/src/typescript.rs b/crates/languages/src/typescript.rs index 47716db38e1930ae5488d004f9dad4a54dc4f3bd..9822ab6f3fc9bcce73e854ae4958f43e23006b39 100644 --- a/crates/languages/src/typescript.rs +++ b/crates/languages/src/typescript.rs @@ -1053,6 +1053,7 @@ mod tests { .unindent(); let buffer = cx.new(|cx| language::Buffer::local(text, cx).with_language(language, cx)); + cx.run_until_parked(); let outline = buffer.read_with(cx, |buffer, _| buffer.snapshot().outline(None)); assert_eq!( outline