From 8fbc88b7087170246d345e88c95c225e4d89b251 Mon Sep 17 00:00:00 2001 From: Piotr Osiewicz <24362066+osiewicz@users.noreply.github.com> Date: Mon, 29 Jan 2024 10:06:57 +0100 Subject: [PATCH] Load embedded fonts directly from .rdata instead of cloning (#6932) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The fonts we embed in Zed binary (Zed Sans & Zed Mono) weigh about 30Mb in total and we are cloning them several times during startup and loading of embedded assets (once explicitly in Zed and then under the hood in font-kit). Moreover, after loading we have at least 2 copies of each font in our program; one in .rdata and the other on the heap for use by font-kit. This commit does away with that distinction (we're no longer allocating the font data) and slightly relaxes the interface of `TextSystem::add_fonts` by expecting one to pass `Cow<[u8]>` instead of `Arc>`. Additionally, `AssetSource::get` now returns `Cow<'static, [u8]>` instead of `Cow<'self, [u8]>`; all existing implementations conform with that change. Note that this optimization takes effect only in Release builds, as the library we use for asset embedding - rust-embed - embeds the assets only in Release mode and in Dev builds it simply loads data from disk. Thus it returns `Cow<[u8]>` in it's interface. Therefore, we still copy that memory around in Dev builds, but that's not really an issue. This patch makes no assumptions about the build profile we're running under, that's just an intrinsic property of rust-embed. Tl;dr: this should shave off about 30Mb of memory usage and a fair chunk (~30ms) of startup time. Release Notes: - Improved startup time and memory usage. --- crates/assets/src/lib.rs | 2 +- crates/gpui/src/assets.rs | 4 +-- crates/gpui/src/platform.rs | 2 +- crates/gpui/src/platform/mac/text_system.rs | 27 +++++++++++++++------ crates/gpui/src/text_system.rs | 3 ++- crates/storybook/src/assets.rs | 2 +- crates/storybook/src/storybook.rs | 8 +++--- crates/theme_importer/src/assets.rs | 2 +- crates/zed/src/main.rs | 6 ++--- crates/zed/src/zed.rs | 14 +++-------- 10 files changed, 36 insertions(+), 34 deletions(-) diff --git a/crates/assets/src/lib.rs b/crates/assets/src/lib.rs index 010b7ebda3d1be4532f47d6b5e7cdb79088694e2..4f013dd5af8cc27e799713b1ea507d8ea297a0fd 100644 --- a/crates/assets/src/lib.rs +++ b/crates/assets/src/lib.rs @@ -16,7 +16,7 @@ use rust_embed::RustEmbed; pub struct Assets; impl AssetSource for Assets { - fn load(&self, path: &str) -> Result> { + fn load(&self, path: &str) -> Result> { Self::get(path) .map(|f| f.data) .ok_or_else(|| anyhow!("could not find asset at path \"{}\"", path)) diff --git a/crates/gpui/src/assets.rs b/crates/gpui/src/assets.rs index b5e3735eb585914c8e88c47660f82c214059e041..e2667e67bbbc8d14e6717f16992cc24ac54f5260 100644 --- a/crates/gpui/src/assets.rs +++ b/crates/gpui/src/assets.rs @@ -11,14 +11,14 @@ use std::{ /// A source of assets for this app to use. pub trait AssetSource: 'static + Send + Sync { /// Load the given asset from the source path. - fn load(&self, path: &str) -> Result>; + fn load(&self, path: &str) -> Result>; /// List the assets at the given path. fn list(&self, path: &str) -> Result>; } impl AssetSource for () { - fn load(&self, path: &str) -> Result> { + fn load(&self, path: &str) -> Result> { Err(anyhow!( "get called on empty asset provider with \"{}\"", path diff --git a/crates/gpui/src/platform.rs b/crates/gpui/src/platform.rs index a7b71c78858366eda21abddf1e8be57ffec78626..92dd08f0598c3bd2a9683a1ba39e30a50d0139dc 100644 --- a/crates/gpui/src/platform.rs +++ b/crates/gpui/src/platform.rs @@ -204,7 +204,7 @@ pub trait PlatformDispatcher: Send + Sync { } pub(crate) trait PlatformTextSystem: Send + Sync { - fn add_fonts(&self, fonts: &[Arc>]) -> Result<()>; + fn add_fonts(&self, fonts: Vec>) -> Result<()>; fn all_font_names(&self) -> Vec; fn all_font_families(&self) -> Vec; fn font_id(&self, descriptor: &Font) -> Result; diff --git a/crates/gpui/src/platform/mac/text_system.rs b/crates/gpui/src/platform/mac/text_system.rs index e8c8b45311e628fe96549fca5a4f9a10b399473e..053e5859be07acddc93e35a1464daa391dd63b85 100644 --- a/crates/gpui/src/platform/mac/text_system.rs +++ b/crates/gpui/src/platform/mac/text_system.rs @@ -34,7 +34,7 @@ use pathfinder_geometry::{ vector::{Vector2F, Vector2I}, }; use smallvec::SmallVec; -use std::{char, cmp, convert::TryFrom, ffi::c_void, sync::Arc}; +use std::{borrow::Cow, char, cmp, convert::TryFrom, ffi::c_void, sync::Arc}; use super::open_type; @@ -74,7 +74,7 @@ impl Default for MacTextSystem { } impl PlatformTextSystem for MacTextSystem { - fn add_fonts(&self, fonts: &[Arc>]) -> Result<()> { + fn add_fonts(&self, fonts: Vec>) -> Result<()> { self.0.write().add_fonts(fonts) } @@ -183,12 +183,23 @@ impl PlatformTextSystem for MacTextSystem { } impl MacTextSystemState { - fn add_fonts(&mut self, fonts: &[Arc>]) -> Result<()> { - self.memory_source.add_fonts( - fonts - .iter() - .map(|bytes| Handle::from_memory(bytes.clone(), 0)), - )?; + fn add_fonts(&mut self, fonts: Vec>) -> Result<()> { + let fonts = fonts + .into_iter() + .map(|bytes| match bytes { + Cow::Borrowed(embedded_font) => { + let data_provider = unsafe { + core_graphics::data_provider::CGDataProvider::from_slice(embedded_font) + }; + let font = core_graphics::font::CGFont::from_data_provider(data_provider) + .map_err(|_| anyhow!("Could not load an embedded font."))?; + let font = font_kit::loaders::core_text::Font::from_core_graphics_font(font); + Ok(Handle::from_native(&font)) + } + Cow::Owned(bytes) => Ok(Handle::from_memory(Arc::new(bytes), 0)), + }) + .collect::>>()?; + self.memory_source.add_fonts(fonts.into_iter())?; Ok(()) } diff --git a/crates/gpui/src/text_system.rs b/crates/gpui/src/text_system.rs index 15042cbf596c99b4707cc6d4e787aad5f0072765..d5180e30c07fb05a9f8f19bcb786487d130821cb 100644 --- a/crates/gpui/src/text_system.rs +++ b/crates/gpui/src/text_system.rs @@ -19,6 +19,7 @@ use itertools::Itertools; use parking_lot::{Mutex, RwLock, RwLockUpgradableReadGuard}; use smallvec::{smallvec, SmallVec}; use std::{ + borrow::Cow, cmp, fmt::{Debug, Display, Formatter}, hash::{Hash, Hasher}, @@ -85,7 +86,7 @@ impl TextSystem { } /// Add a font's data to the text system. - pub fn add_fonts(&self, fonts: &[Arc>]) -> Result<()> { + pub fn add_fonts(&self, fonts: Vec>) -> Result<()> { self.platform_text_system.add_fonts(fonts) } diff --git a/crates/storybook/src/assets.rs b/crates/storybook/src/assets.rs index 9fc71917b45f7fa024a3e157f0a806d6056ef7f4..c8dbe9ec606ee639e945fe84ec218346a474df96 100644 --- a/crates/storybook/src/assets.rs +++ b/crates/storybook/src/assets.rs @@ -15,7 +15,7 @@ use rust_embed::RustEmbed; pub struct Assets; impl AssetSource for Assets { - fn load(&self, path: &str) -> Result> { + fn load(&self, path: &str) -> Result> { Self::get(path) .map(|f| f.data) .ok_or_else(|| anyhow!("could not find asset at path \"{}\"", path)) diff --git a/crates/storybook/src/storybook.rs b/crates/storybook/src/storybook.rs index 70e830222fcd4cc43178c83fa2fd78051220caeb..47fc3b042a9ae5466c442ab9f79bdfea0b67eaf1 100644 --- a/crates/storybook/src/storybook.rs +++ b/crates/storybook/src/storybook.rs @@ -2,8 +2,6 @@ mod assets; mod stories; mod story_selector; -use std::sync::Arc; - use clap::Parser; use dialoguer::FuzzySelect; use gpui::{ @@ -128,10 +126,10 @@ fn load_embedded_fonts(cx: &AppContext) -> gpui::Result<()> { let mut embedded_fonts = Vec::new(); for font_path in font_paths { if font_path.ends_with(".ttf") { - let font_bytes = cx.asset_source().load(&font_path)?.to_vec(); - embedded_fonts.push(Arc::from(font_bytes)); + let font_bytes = cx.asset_source().load(&font_path)?; + embedded_fonts.push(font_bytes); } } - cx.text_system().add_fonts(&embedded_fonts) + cx.text_system().add_fonts(embedded_fonts) } diff --git a/crates/theme_importer/src/assets.rs b/crates/theme_importer/src/assets.rs index 9009b4c144cd7b73132a7cc3246bab627e4894bc..171912f6b828da5b5c1f48650df02543e40779d7 100644 --- a/crates/theme_importer/src/assets.rs +++ b/crates/theme_importer/src/assets.rs @@ -11,7 +11,7 @@ use rust_embed::RustEmbed; pub struct Assets; impl AssetSource for Assets { - fn load(&self, path: &str) -> Result> { + fn load(&self, path: &str) -> Result> { Self::get(path) .map(|f| f.data) .ok_or_else(|| anyhow!("could not find asset at path \"{}\"", path)) diff --git a/crates/zed/src/main.rs b/crates/zed/src/main.rs index 0b7a983cb68e6f6cbe777320b7cee7d0006ce3df..3e546ca5470c55e6d6c0de800c18d23a588c57d6 100644 --- a/crates/zed/src/main.rs +++ b/crates/zed/src/main.rs @@ -815,14 +815,14 @@ fn load_embedded_fonts(cx: &AppContext) { } scope.spawn(async { - let font_bytes = asset_source.load(font_path).unwrap().to_vec(); - embedded_fonts.lock().push(Arc::from(font_bytes)); + let font_bytes = asset_source.load(font_path).unwrap(); + embedded_fonts.lock().push(font_bytes); }); } })); cx.text_system() - .add_fonts(&embedded_fonts.into_inner()) + .add_fonts(embedded_fonts.into_inner()) .unwrap(); } diff --git a/crates/zed/src/zed.rs b/crates/zed/src/zed.rs index bf001dac72e1fd733523e0b6304b5541a0e3aa64..f045e62f8352c72ba43748e79620bd6f150a554c 100644 --- a/crates/zed/src/zed.rs +++ b/crates/zed/src/zed.rs @@ -2676,17 +2676,9 @@ mod tests { #[gpui::test] fn test_bundled_settings_and_themes(cx: &mut AppContext) { cx.text_system() - .add_fonts(&[ - Assets - .load("fonts/zed-sans/zed-sans-extended.ttf") - .unwrap() - .to_vec() - .into(), - Assets - .load("fonts/zed-mono/zed-mono-extended.ttf") - .unwrap() - .to_vec() - .into(), + .add_fonts(vec![ + Assets.load("fonts/zed-sans/zed-sans-extended.ttf").unwrap(), + Assets.load("fonts/zed-mono/zed-mono-extended.ttf").unwrap(), ]) .unwrap(); let themes = ThemeRegistry::default();