Move pooling of line wrappers into `FontCache`

Antonio Scandurra and Nathan Sobo created

Co-Authored-By: Nathan Sobo <nathan@zed.dev>

Change summary

gpui/src/elements/text.rs              |  4 
gpui/src/font_cache.rs                 | 62 ++++++++++++++++++++++++++-
gpui/src/presenter.rs                  |  2 
gpui/src/text_layout.rs                | 50 ---------------------
zed/src/editor/display_map/wrap_map.rs |  7 --
5 files changed, 65 insertions(+), 60 deletions(-)

Detailed changes

gpui/src/elements/text.rs 🔗

@@ -7,7 +7,7 @@ use crate::{
         vector::{vec2f, Vector2F},
     },
     json::{ToJson, Value},
-    text_layout::{Line, LineWrapper, ShapedBoundary},
+    text_layout::{Line, ShapedBoundary},
     DebugContext, Element, Event, EventContext, LayoutContext, PaintContext, SizeConstraint,
 };
 use serde_json::json;
@@ -60,7 +60,7 @@ impl Element for Text {
             .unwrap();
         let line_height = cx.font_cache.line_height(font_id, self.font_size);
 
-        let mut wrapper = LineWrapper::acquire(font_id, self.font_size, cx.font_system.clone());
+        let mut wrapper = cx.font_cache.line_wrapper(font_id, self.font_size);
         let mut lines = Vec::new();
         let mut line_count = 0;
         let mut max_line_width = 0_f32;

gpui/src/font_cache.rs 🔗

@@ -2,10 +2,16 @@ use crate::{
     fonts::{FontId, Metrics, Properties},
     geometry::vector::{vec2f, Vector2F},
     platform,
+    text_layout::LineWrapper,
 };
 use anyhow::{anyhow, Result};
+use ordered_float::OrderedFloat;
 use parking_lot::{RwLock, RwLockUpgradableReadGuard};
-use std::{collections::HashMap, sync::Arc};
+use std::{
+    collections::HashMap,
+    ops::{Deref, DerefMut},
+    sync::Arc,
+};
 
 #[derive(Copy, Clone, Debug, Eq, PartialEq, Hash)]
 pub struct FamilyId(usize);
@@ -22,6 +28,12 @@ pub struct FontCacheState {
     families: Vec<Family>,
     font_selections: HashMap<FamilyId, HashMap<Properties, FontId>>,
     metrics: HashMap<FontId, Metrics>,
+    wrapper_pool: HashMap<(FontId, OrderedFloat<f32>), Vec<LineWrapper>>,
+}
+
+pub struct LineWrapperHandle {
+    wrapper: Option<LineWrapper>,
+    font_cache: Arc<FontCache>,
 }
 
 unsafe impl Send for FontCache {}
@@ -30,9 +42,10 @@ impl FontCache {
     pub fn new(fonts: Arc<dyn platform::FontSystem>) -> Self {
         Self(RwLock::new(FontCacheState {
             fonts,
-            families: Vec::new(),
-            font_selections: HashMap::new(),
-            metrics: HashMap::new(),
+            families: Default::default(),
+            font_selections: Default::default(),
+            metrics: Default::default(),
+            wrapper_pool: Default::default(),
         }))
     }
 
@@ -160,6 +173,47 @@ impl FontCache {
     pub fn scale_metric(&self, metric: f32, font_id: FontId, font_size: f32) -> f32 {
         metric * font_size / self.metric(font_id, |m| m.units_per_em as f32)
     }
+
+    pub fn line_wrapper(self: &Arc<Self>, font_id: FontId, font_size: f32) -> LineWrapperHandle {
+        let mut state = self.0.write();
+        let wrappers = state
+            .wrapper_pool
+            .entry((font_id, OrderedFloat(font_size)))
+            .or_default();
+        let wrapper = wrappers
+            .pop()
+            .unwrap_or_else(|| LineWrapper::new(font_id, font_size, state.fonts.clone()));
+        LineWrapperHandle {
+            wrapper: Some(wrapper),
+            font_cache: self.clone(),
+        }
+    }
+}
+
+impl Drop for LineWrapperHandle {
+    fn drop(&mut self) {
+        let mut state = self.font_cache.0.write();
+        let wrapper = self.wrapper.take().unwrap();
+        state
+            .wrapper_pool
+            .get_mut(&(wrapper.font_id, OrderedFloat(wrapper.font_size)))
+            .unwrap()
+            .push(wrapper);
+    }
+}
+
+impl Deref for LineWrapperHandle {
+    type Target = LineWrapper;
+
+    fn deref(&self) -> &Self::Target {
+        self.wrapper.as_ref().unwrap()
+    }
+}
+
+impl DerefMut for LineWrapperHandle {
+    fn deref_mut(&mut self) -> &mut Self::Target {
+        self.wrapper.as_mut().unwrap()
+    }
 }
 
 #[cfg(test)]

gpui/src/presenter.rs 🔗

@@ -173,7 +173,7 @@ pub struct LayoutContext<'a> {
     rendered_views: &'a mut HashMap<usize, ElementBox>,
     parents: &'a mut HashMap<usize, usize>,
     view_stack: Vec<usize>,
-    pub font_cache: &'a FontCache,
+    pub font_cache: &'a Arc<FontCache>,
     pub font_system: Arc<dyn FontSystem>,
     pub text_layout_cache: &'a TextLayoutCache,
     pub asset_cache: &'a AssetCache,

gpui/src/text_layout.rs 🔗

@@ -7,7 +7,6 @@ use crate::{
     },
     platform, scene, FontSystem, PaintContext,
 };
-use lazy_static::lazy_static;
 use ordered_float::OrderedFloat;
 use parking_lot::{Mutex, RwLock, RwLockUpgradableReadGuard};
 use smallvec::SmallVec;
@@ -16,7 +15,6 @@ use std::{
     collections::HashMap,
     hash::{Hash, Hasher},
     iter,
-    ops::{Deref, DerefMut},
     sync::Arc,
 };
 
@@ -307,10 +305,6 @@ impl Run {
     }
 }
 
-lazy_static! {
-    static ref WRAPPER_POOL: Mutex<Vec<LineWrapper>> = Default::default();
-}
-
 #[derive(Copy, Clone, Debug, PartialEq, Eq)]
 pub struct Boundary {
     pub ix: usize,
@@ -331,8 +325,8 @@ impl Boundary {
 
 pub struct LineWrapper {
     font_system: Arc<dyn FontSystem>,
-    font_id: FontId,
-    font_size: f32,
+    pub(crate) font_id: FontId,
+    pub(crate) font_size: f32,
     cached_ascii_char_widths: [f32; 128],
     cached_other_char_widths: HashMap<char, f32>,
 }
@@ -340,23 +334,6 @@ pub struct LineWrapper {
 impl LineWrapper {
     pub const MAX_INDENT: u32 = 256;
 
-    pub fn acquire(
-        font_id: FontId,
-        font_size: f32,
-        font_system: Arc<dyn FontSystem>,
-    ) -> LineWrapperHandle {
-        let wrapper = if let Some(mut wrapper) = WRAPPER_POOL.lock().pop() {
-            if wrapper.font_id != font_id || wrapper.font_size != font_size {
-                wrapper.cached_ascii_char_widths = [f32::NAN; 128];
-                wrapper.cached_other_char_widths.clear();
-            }
-            wrapper
-        } else {
-            LineWrapper::new(font_id, font_size, font_system)
-        };
-        LineWrapperHandle(Some(wrapper))
-    }
-
     pub fn new(font_id: FontId, font_size: f32, font_system: Arc<dyn FontSystem>) -> Self {
         Self {
             font_system,
@@ -534,29 +511,6 @@ impl LineWrapper {
     }
 }
 
-pub struct LineWrapperHandle(Option<LineWrapper>);
-
-impl Drop for LineWrapperHandle {
-    fn drop(&mut self) {
-        let wrapper = self.0.take().unwrap();
-        WRAPPER_POOL.lock().push(wrapper)
-    }
-}
-
-impl Deref for LineWrapperHandle {
-    type Target = LineWrapper;
-
-    fn deref(&self) -> &Self::Target {
-        self.0.as_ref().unwrap()
-    }
-}
-
-impl DerefMut for LineWrapperHandle {
-    fn deref_mut(&mut self) -> &mut Self::Target {
-        self.0.as_mut().unwrap()
-    }
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;

zed/src/editor/display_map/wrap_map.rs 🔗

@@ -115,15 +115,13 @@ impl WrapMap {
 
         if let Some(wrap_width) = wrap_width {
             let mut new_snapshot = self.snapshot.clone();
-            let font_system = cx.platform().fonts();
             let font_cache = cx.font_cache().clone();
             let settings = self.settings.clone();
             let task = cx.background().spawn(async move {
                 let font_id = font_cache
                     .select_font(settings.buffer_font_family, &Default::default())
                     .unwrap();
-                let mut line_wrapper =
-                    LineWrapper::acquire(font_id, settings.buffer_font_size, font_system);
+                let mut line_wrapper = font_cache.line_wrapper(font_id, settings.buffer_font_size);
                 let tab_snapshot = new_snapshot.tab_snapshot.clone();
                 let range = TabPoint::zero()..tab_snapshot.max_point();
                 new_snapshot
@@ -193,7 +191,6 @@ impl WrapMap {
             if self.background_task.is_none() {
                 let pending_edits = self.pending_edits.clone();
                 let mut snapshot = self.snapshot.clone();
-                let font_system = cx.platform().fonts();
                 let font_cache = cx.font_cache().clone();
                 let settings = self.settings.clone();
                 let update_task = cx.background().spawn(async move {
@@ -201,7 +198,7 @@ impl WrapMap {
                         .select_font(settings.buffer_font_family, &Default::default())
                         .unwrap();
                     let mut line_wrapper =
-                        LineWrapper::acquire(font_id, settings.buffer_font_size, font_system);
+                        font_cache.line_wrapper(font_id, settings.buffer_font_size);
                     for (tab_snapshot, edits) in pending_edits {
                         snapshot
                             .update(tab_snapshot, &edits, wrap_width, &mut line_wrapper)