From 9da8142e2ac7db3e7c19a098b296f57ab3a85533 Mon Sep 17 00:00:00 2001 From: Josh Matthews Date: Sun, 17 Aug 2025 14:59:30 -0400 Subject: [PATCH] fonts: Measure more FontContext heap usage. (#38733) Replace a hand-written MallocSizeOf implementation with an automatically derived one. This exposes more than 1MB of previously-untracked heap data on servo.org. Testing: Compared about:memory output for servo.org before and after. Part of: #11559 Signed-off-by: Josh Matthews --- Cargo.lock | 1 + components/fonts/font.rs | 11 +++++- components/fonts/font_context.rs | 47 +++++++++++-------------- components/fonts/font_store.rs | 12 ++++++- components/fonts/lib.rs | 5 +-- components/fonts/system_font_service.rs | 2 +- components/malloc_size_of/Cargo.toml | 1 + components/malloc_size_of/lib.rs | 12 +++++++ 8 files changed, 59 insertions(+), 32 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 21891223644..11e85bdafd8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7715,6 +7715,7 @@ dependencies = [ "keyboard-types", "markup5ever", "mime", + "parking_lot", "resvg", "servo_allocator", "servo_arc", diff --git a/components/fonts/font.rs b/components/fonts/font.rs index 8723979814a..1a323d6b36a 100644 --- a/components/fonts/font.rs +++ b/components/fonts/font.rs @@ -4,6 +4,7 @@ use std::borrow::ToOwned; use std::collections::HashMap; +use std::ops::Deref; use std::sync::atomic::{AtomicUsize, Ordering}; use std::sync::{Arc, OnceLock}; use std::time::Instant; @@ -548,7 +549,15 @@ impl Font { } } -pub type FontRef = Arc; +#[derive(Clone, MallocSizeOf)] +pub struct FontRef(#[conditional_malloc_size_of] pub(crate) Arc); + +impl Deref for FontRef { + type Target = Arc; + fn deref(&self) -> &Self::Target { + &self.0 + } +} /// A `FontGroup` is a prioritised list of fonts for a given set of font styles. It is used by /// `TextRun` to decide which font to render a character with. If none of the fonts listed in the diff --git a/components/fonts/font_context.rs b/components/fonts/font_context.rs index 01b7e2a0977..ff080cebae8 100644 --- a/components/fonts/font_context.rs +++ b/components/fonts/font_context.rs @@ -5,6 +5,7 @@ use std::collections::{HashMap, HashSet}; use std::default::Default; use std::hash::{BuildHasherDefault, Hash, Hasher}; +use std::ops::Deref; use std::sync::Arc; use std::sync::atomic::{AtomicBool, Ordering}; @@ -14,7 +15,6 @@ use compositing_traits::CrossProcessCompositorApi; use fnv::FnvHasher; use fonts_traits::StylesheetWebFontLoadFinishedCallback; use log::{debug, trace}; -use malloc_size_of::{MallocSizeOf, MallocSizeOfOps}; use malloc_size_of_derive::MallocSizeOf; use net_traits::request::{Destination, Referrer, RequestBuilder}; use net_traits::{CoreResourceThread, FetchResponseMsg, ResourceThreads, fetch_async}; @@ -45,12 +45,25 @@ use crate::{FontData, LowercaseFontFamilyName, PlatformFontMethods, SystemFontSe static SMALL_CAPS_SCALE_FACTOR: f32 = 0.8; // Matches FireFox (see gfxFont.h) +#[derive(MallocSizeOf)] +struct FontGroupRef(#[conditional_malloc_size_of] Arc>); + +impl Deref for FontGroupRef { + type Target = Arc>; + fn deref(&self) -> &Self::Target { + &self.0 + } +} + /// The FontContext represents the per-thread/thread state necessary for /// working with fonts. It is the public API used by the layout and /// paint code. It talks directly to the system font service where /// required. +#[derive(MallocSizeOf)] pub struct FontContext { + #[conditional_malloc_size_of] system_font_service_proxy: Arc, + resource_threads: Mutex, /// A sender that can send messages and receive replies from the compositor. @@ -64,7 +77,7 @@ pub struct FontContext { /// resolved [`FontGroup`] which contains information about all fonts that /// can be selected with that style. resolved_font_groups: - RwLock>, BuildHasherDefault>>, + RwLock>>, web_fonts: CrossThreadFontStore, @@ -83,26 +96,6 @@ pub struct FontContext { have_removed_web_fonts: AtomicBool, } -impl MallocSizeOf for FontContext { - fn size_of(&self, ops: &mut MallocSizeOfOps) -> usize { - let font_cache_size = self - .fonts - .read() - .iter() - .map(|(key, font)| { - key.size_of(ops) + font.as_ref().map_or(0, |font| (*font).size_of(ops)) - }) - .sum::(); - let font_group_cache_size = self - .resolved_font_groups - .read() - .iter() - .map(|(key, font_group)| key.size_of(ops) + (*font_group.read()).size_of(ops)) - .sum::(); - font_cache_size + font_group_cache_size - } -} - impl FontContext { pub fn new( system_font_service_proxy: Arc, @@ -116,7 +109,7 @@ impl FontContext { compositor_api: Mutex::new(compositor_api), fonts: Default::default(), resolved_font_groups: Default::default(), - web_fonts: Arc::new(RwLock::default()), + web_fonts: Default::default(), webrender_font_keys: RwLock::default(), webrender_font_instance_keys: RwLock::default(), have_removed_web_fonts: AtomicBool::new(false), @@ -152,7 +145,7 @@ impl FontContext { ) -> Arc> { let cache_key = FontGroupCacheKey { size, style }; if let Some(font_group) = self.resolved_font_groups.read().get(&cache_key) { - return font_group.clone(); + return font_group.0.clone(); } let mut descriptor = FontDescriptor::from(&*cache_key.style); @@ -161,7 +154,7 @@ impl FontContext { let font_group = Arc::new(RwLock::new(FontGroup::new(&cache_key.style, descriptor))); self.resolved_font_groups .write() - .insert(cache_key, font_group.clone()); + .insert(cache_key, FontGroupRef(font_group.clone())); font_group } @@ -275,12 +268,12 @@ impl FontContext { font_descriptor: FontDescriptor, synthesized_small_caps: Option, ) -> Result { - Ok(Arc::new(Font::new( + Ok(FontRef(Arc::new(Font::new( font_template.clone(), font_descriptor.clone(), self.get_font_data(&font_template.identifier()), synthesized_small_caps, - )?)) + )?))) } pub(crate) fn create_font_instance_key(&self, font: &Font) -> FontInstanceKey { diff --git a/components/fonts/font_store.rs b/components/fonts/font_store.rs index 0099c56c266..16440982e98 100644 --- a/components/fonts/font_store.rs +++ b/components/fonts/font_store.rs @@ -3,6 +3,7 @@ * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ use std::collections::HashMap; +use std::ops::Deref; use std::sync::Arc; use log::warn; @@ -21,7 +22,16 @@ pub struct FontStore { web_fonts_loading_for_stylesheets: Vec<(DocumentStyleSheet, usize)>, web_fonts_loading_for_script: usize, } -pub(crate) type CrossThreadFontStore = Arc>; + +#[derive(Default, MallocSizeOf)] +pub(crate) struct CrossThreadFontStore(#[conditional_malloc_size_of] Arc>); + +impl Deref for CrossThreadFontStore { + type Target = Arc>; + fn deref(&self) -> &Self::Target { + &self.0 + } +} impl FontStore { pub(crate) fn clear(&mut self) { diff --git a/components/fonts/lib.rs b/components/fonts/lib.rs index 3abe9558af9..37b1da91724 100644 --- a/components/fonts/lib.rs +++ b/components/fonts/lib.rs @@ -22,6 +22,7 @@ pub use font_store::*; pub use font_template::*; pub use glyph::*; use ipc_channel::ipc::IpcSharedMemory; +use malloc_size_of_derive::MallocSizeOf; pub use platform::LocalFontIdentifier; pub use shaper::*; pub use system_font_service::*; @@ -30,8 +31,8 @@ use unicode_properties::{EmojiStatus, UnicodeEmoji, emoji}; /// A data structure to store data for fonts. Data is stored internally in an /// [`IpcSharedMemory`] handle, so that it can be send without serialization /// across IPC channels. -#[derive(Clone)] -pub struct FontData(pub(crate) Arc); +#[derive(Clone, MallocSizeOf)] +pub struct FontData(#[conditional_malloc_size_of] pub(crate) Arc); impl FontData { pub fn from_bytes(bytes: &[u8]) -> Self { diff --git a/components/fonts/system_font_service.rs b/components/fonts/system_font_service.rs index ec5fe0e77f2..c41a6b799e5 100644 --- a/components/fonts/system_font_service.rs +++ b/components/fonts/system_font_service.rs @@ -363,7 +363,7 @@ struct FontTemplateCacheKey { /// The public interface to the [`SystemFontService`], used by per-Document /// `FontContext` instances. -#[derive(Debug)] +#[derive(Debug, MallocSizeOf)] pub struct SystemFontServiceProxy { sender: Mutex>, templates: RwLock>>, diff --git a/components/malloc_size_of/Cargo.toml b/components/malloc_size_of/Cargo.toml index de2266b3c00..6e5e45a4e35 100644 --- a/components/malloc_size_of/Cargo.toml +++ b/components/malloc_size_of/Cargo.toml @@ -23,6 +23,7 @@ ipc-channel = { workspace = true } keyboard-types = { workspace = true } markup5ever = { workspace = true } mime = { workspace = true } +parking_lot = { workspace = true } resvg = { workspace = true } servo_allocator = { path = "../allocator" } servo_arc = { workspace = true } diff --git a/components/malloc_size_of/lib.rs b/components/malloc_size_of/lib.rs index e156b2ceb5c..29fa3250fdd 100644 --- a/components/malloc_size_of/lib.rs +++ b/components/malloc_size_of/lib.rs @@ -613,6 +613,18 @@ impl MallocSizeOf for std::sync::Mutex { } } +impl MallocSizeOf for parking_lot::Mutex { + fn size_of(&self, ops: &mut MallocSizeOfOps) -> usize { + (*self.lock()).size_of(ops) + } +} + +impl MallocSizeOf for parking_lot::RwLock { + fn size_of(&self, ops: &mut MallocSizeOfOps) -> usize { + (*self.read()).size_of(ops) + } +} + impl MallocSizeOf for euclid::Length { fn size_of(&self, ops: &mut MallocSizeOfOps) -> usize { self.0.size_of(ops)