From 81cd435e08d395bf6df91b7c3d8fc5751e46ea93 Mon Sep 17 00:00:00 2001 From: Finn Evers Date: Mon, 13 Oct 2025 11:53:19 +0200 Subject: [PATCH] Improve loading times for extension themes (#40015) This PR primarily does two things: - replace `serde_json::from_reader` with `serde_json::from_slice`, as the latter is much much faster, even with loading the file into memory first. - runs the initial loading of themes and icon themes coming from extensions in parallel instead of sequential. Measuring the `eager_load_active_theme_and_icon_theme` method, this drastically improves the speed at which this happens (tested this method primarily with debug builds on my MacBook Pro, but the `Before` measurement was also confirmed against a `release-fast` build): - Before: ~260ms on average (in one run, it even took 600ms) - After: ~20ms on average Which reduces the time this method takes to load these by around ~92%. Given that we block on this during the initial app startup, this should drastically improve Zeds initial startup loading time. Yet, it also improves responsiveness when installing theme extensions and trying these. I also replaced all other `serde_json::from_reader` implementations with `serde_json::from_slice` and added the former to `disallowed_methods`, given https://github.com/serde-rs/json/issues/160#issuecomment-253446892. Release Notes: - Improved Zed startup speed when using themes provided by extensions --- clippy.toml | 2 + crates/diagnostics/src/diagnostics.rs | 25 ++++-- crates/theme/src/theme.rs | 8 +- crates/theme_importer/src/main.rs | 9 +- crates/zed/src/zed.rs | 114 ++++++++++++++++++-------- tooling/perf/src/main.rs | 12 ++- 6 files changed, 115 insertions(+), 55 deletions(-) diff --git a/clippy.toml b/clippy.toml index 57f6f59385..0976e2eba3 100644 --- a/clippy.toml +++ b/clippy.toml @@ -9,6 +9,8 @@ disallowed-methods = [ { path = "std::process::Command::spawn", reason = "Spawning `std::process::Command` can block the current thread for an unknown duration", replacement = "smol::process::Command::spawn" }, { path = "std::process::Command::output", reason = "Spawning `std::process::Command` can block the current thread for an unknown duration", replacement = "smol::process::Command::output" }, { path = "std::process::Command::status", reason = "Spawning `std::process::Command` can block the current thread for an unknown duration", replacement = "smol::process::Command::status" }, + { path = "serde_json::from_reader", reason = "Parsing from a buffer is much slower than first reading the buffer into a Vec/String, see https://github.com/serde-rs/json/issues/160#issuecomment-253446892. Use `serde_json::from_slice` instead." }, + { path = "serde_json_lenient::from_reader", reason = "Parsing from a buffer is much slower than first reading the buffer into a Vec/String, see https://github.com/serde-rs/json/issues/160#issuecomment-253446892, Use `serde_json_lenient::from_slice` instead." }, ] disallowed-types = [ # { path = "std::collections::HashMap", replacement = "collections::HashMap" }, diff --git a/crates/diagnostics/src/diagnostics.rs b/crates/diagnostics/src/diagnostics.rs index 5fbd958141..47e2a9539b 100644 --- a/crates/diagnostics/src/diagnostics.rs +++ b/crates/diagnostics/src/diagnostics.rs @@ -965,10 +965,11 @@ async fn heuristic_syntactic_expand( let row_count = node_end.row - node_start.row + 1; let mut ancestor_range = None; let reached_outline_node = cx.background_executor().scoped({ - let node_range = node_range.clone(); - let outline_range = outline_range.clone(); - let ancestor_range = &mut ancestor_range; - |scope| {scope.spawn(async move { + let node_range = node_range.clone(); + let outline_range = outline_range.clone(); + let ancestor_range = &mut ancestor_range; + |scope| { + scope.spawn(async move { // Stop if we've exceeded the row count or reached an outline node. Then, find the interval // of node children which contains the query range. For example, this allows just returning // the header of a declaration rather than the entire declaration. @@ -980,8 +981,11 @@ async fn heuristic_syntactic_expand( if cursor.goto_first_child() { loop { let child_node = cursor.node(); - let child_range = previous_end..Point::from_ts_point(child_node.end_position()); - if included_child_start.is_none() && child_range.contains(&input_range.start) { + let child_range = + previous_end..Point::from_ts_point(child_node.end_position()); + if included_child_start.is_none() + && child_range.contains(&input_range.start) + { included_child_start = Some(child_range.start); } if child_range.contains(&input_range.end) { @@ -997,19 +1001,22 @@ async fn heuristic_syntactic_expand( if let Some(start) = included_child_start { let row_count = end.row - start.row; if row_count < max_row_count { - *ancestor_range = Some(Some(RangeInclusive::new(start.row, end.row))); + *ancestor_range = + Some(Some(RangeInclusive::new(start.row, end.row))); return; } } log::info!( - "Expanding to ancestor started on {} node exceeding row limit of {max_row_count}.", + "Expanding to ancestor started on {} node\ + exceeding row limit of {max_row_count}.", node.grammar_name() ); *ancestor_range = Some(None); } }) - }}); + } + }); reached_outline_node.await; if let Some(node) = ancestor_range { return node; diff --git a/crates/theme/src/theme.rs b/crates/theme/src/theme.rs index c18719efe0..fab173484d 100644 --- a/crates/theme/src/theme.rs +++ b/crates/theme/src/theme.rs @@ -408,8 +408,8 @@ impl Theme { /// Asynchronously reads the user theme from the specified path. pub async fn read_user_theme(theme_path: &Path, fs: Arc) -> Result { - let reader = fs.open_sync(theme_path).await?; - let theme_family: ThemeFamilyContent = serde_json_lenient::from_reader(reader)?; + let bytes = fs.load_bytes(theme_path).await?; + let theme_family: ThemeFamilyContent = serde_json_lenient::from_slice(&bytes)?; for theme in &theme_family.themes { if theme @@ -433,8 +433,8 @@ pub async fn read_icon_theme( icon_theme_path: &Path, fs: Arc, ) -> Result { - let reader = fs.open_sync(icon_theme_path).await?; - let icon_theme_family: IconThemeFamilyContent = serde_json_lenient::from_reader(reader)?; + let bytes = fs.load_bytes(icon_theme_path).await?; + let icon_theme_family: IconThemeFamilyContent = serde_json_lenient::from_slice(&bytes)?; Ok(icon_theme_family) } diff --git a/crates/theme_importer/src/main.rs b/crates/theme_importer/src/main.rs index 0ea6bbc4bc..24291fc511 100644 --- a/crates/theme_importer/src/main.rs +++ b/crates/theme_importer/src/main.rs @@ -2,7 +2,7 @@ mod color; mod vscode; use std::fs::File; -use std::io::Write; +use std::io::{Read, Write}; use std::path::PathBuf; use anyhow::{Context as _, Result}; @@ -89,15 +89,16 @@ fn main() -> Result<()> { let theme_file_path = args.theme_path; - let theme_file = match File::open(&theme_file_path) { - Ok(file) => file, + let mut buffer = Vec::new(); + match File::open(&theme_file_path).and_then(|mut file| file.read_to_end(&mut buffer)) { + Ok(_) => {} Err(err) => { log::info!("Failed to open file at path: {:?}", theme_file_path); return Err(err)?; } }; - let vscode_theme: VsCodeTheme = serde_json_lenient::from_reader(theme_file) + let vscode_theme: VsCodeTheme = serde_json_lenient::from_slice(&buffer) .context(format!("failed to parse theme {theme_file_path:?}"))?; let theme_metadata = ThemeMetadata { diff --git a/crates/zed/src/zed.rs b/crates/zed/src/zed.rs index 58fb18ebd2..928ffa5baf 100644 --- a/crates/zed/src/zed.rs +++ b/crates/zed/src/zed.rs @@ -70,10 +70,7 @@ use std::{ sync::atomic::{self, AtomicBool}, }; use terminal_view::terminal_panel::{self, TerminalPanel}; -use theme::{ - ActiveTheme, GlobalTheme, IconThemeNotFoundError, SystemAppearance, ThemeNotFoundError, - ThemeRegistry, ThemeSettings, -}; +use theme::{ActiveTheme, GlobalTheme, SystemAppearance, ThemeRegistry, ThemeSettings}; use ui::{PopoverMenuHandle, prelude::*}; use util::markdown::MarkdownString; use util::rel_path::RelPath; @@ -2032,41 +2029,88 @@ pub(crate) fn eager_load_active_theme_and_icon_theme(fs: Arc, cx: &mut A let theme_settings = ThemeSettings::get_global(cx); let appearance = SystemAppearance::global(cx).0; - let theme_name = theme_settings.theme.name(appearance); - if matches!( - theme_registry.get(&theme_name.0), - Err(ThemeNotFoundError(_)) - ) && let Some(theme_path) = extension_store - .read(cx) - .path_to_extension_theme(&theme_name.0) - { - if cx - .background_executor() - .block(theme_registry.load_user_theme(&theme_path, fs.clone())) - .log_err() - .is_some() - { - GlobalTheme::reload_theme(cx); - } + enum LoadTarget { + Theme(PathBuf), + IconTheme((PathBuf, PathBuf)), } - let theme_settings = ThemeSettings::get_global(cx); + let theme_name = theme_settings.theme.name(appearance); let icon_theme_name = theme_settings.icon_theme.name(appearance); - if matches!( - theme_registry.get_icon_theme(&icon_theme_name.0), - Err(IconThemeNotFoundError(_)) - ) && let Some((icon_theme_path, icons_root_path)) = extension_store - .read(cx) - .path_to_extension_icon_theme(&icon_theme_name.0) - { - if cx - .background_executor() - .block(theme_registry.load_icon_theme(&icon_theme_path, &icons_root_path, fs)) - .log_err() - .is_some() - { - GlobalTheme::reload_icon_theme(cx); + let themes_to_load = [ + theme_registry + .get(&theme_name.0) + .is_err() + .then(|| { + extension_store + .read(cx) + .path_to_extension_theme(&theme_name.0) + }) + .flatten() + .map(LoadTarget::Theme), + theme_registry + .get_icon_theme(&icon_theme_name.0) + .is_err() + .then(|| { + extension_store + .read(cx) + .path_to_extension_icon_theme(&icon_theme_name.0) + }) + .flatten() + .map(LoadTarget::IconTheme), + ]; + + enum ReloadTarget { + Theme, + IconTheme, + } + + let executor = cx.background_executor(); + let reload_tasks = parking_lot::Mutex::new(Vec::with_capacity(themes_to_load.len())); + + let mut themes_to_load = themes_to_load.into_iter().flatten().peekable(); + + if themes_to_load.peek().is_none() { + return; + } + + executor.block(executor.scoped(|scope| { + for load_target in themes_to_load { + let theme_registry = &theme_registry; + let reload_tasks = &reload_tasks; + let fs = fs.clone(); + + scope.spawn(async { + match load_target { + LoadTarget::Theme(theme_path) => { + if theme_registry + .load_user_theme(&theme_path, fs) + .await + .log_err() + .is_some() + { + reload_tasks.lock().push(ReloadTarget::Theme); + } + } + LoadTarget::IconTheme((icon_theme_path, icons_root_path)) => { + if theme_registry + .load_icon_theme(&icon_theme_path, &icons_root_path, fs) + .await + .log_err() + .is_some() + { + reload_tasks.lock().push(ReloadTarget::IconTheme); + } + } + } + }); } + })); + + for reload_target in reload_tasks.into_inner() { + match reload_target { + ReloadTarget::Theme => GlobalTheme::reload_theme(cx), + ReloadTarget::IconTheme => GlobalTheme::reload_icon_theme(cx), + }; } } diff --git a/tooling/perf/src/main.rs b/tooling/perf/src/main.rs index 910b172958..63ad9caf3f 100644 --- a/tooling/perf/src/main.rs +++ b/tooling/perf/src/main.rs @@ -50,7 +50,7 @@ use zed_perf::{FailKind, Importance, Output, TestMdata, Timings, consts}; use std::{ fs::OpenOptions, - io::Write, + io::{Read, Write}, num::NonZero, path::{Path, PathBuf}, process::{Command, Stdio}, @@ -264,8 +264,14 @@ fn compare_profiles(args: &[String]) { let prefix = elems.next().unwrap(); assert_eq!("json", elems.next().unwrap()); assert!(elems.next().is_none()); - let handle = OpenOptions::new().read(true).open(entry.path()).unwrap(); - let o_other: Output = serde_json::from_reader(handle).unwrap(); + let mut buffer = Vec::new(); + let _ = OpenOptions::new() + .read(true) + .open(entry.path()) + .unwrap() + .read_to_end(&mut buffer) + .unwrap(); + let o_other: Output = serde_json::from_slice(&buffer).unwrap(); output.merge(o_other, prefix); };