Cache font-missing result to avoid unnecessary lookups

Thorsten Ball and Antonio created 2 years ago

This fixes the performance problem we saw in https://github.com/zed-industries/community/issues/2405.

In a trace we could see that if a font is missing we'd constantly look
it up and never cache that it's missing.

This changes that and does cache the font-is-missing result.

Drawback is that one would need to restart Zed after installing a
missing font that was configured in settings. That seems acceptable for
now, though.

Co-authored-by: Antonio <[e-mail]>

Change summary

crates/gpui/src/text_system.rs | 25 +++++++++++++++++++------
1 file changed, 19 insertions(+), 6 deletions(-)

Detailed changes

crates/gpui/src/text_system.rs

@@ -41,7 +41,7 @@ pub(crate) const SUBPIXEL_VARIANTS: u8 = 4;
 pub struct TextSystem {
     line_layout_cache: Arc<LineLayoutCache>,
     platform_text_system: Arc<dyn PlatformTextSystem>,
-    font_ids_by_font: RwLock<FxHashMap<Font, FontId>>,
+    font_ids_by_font: RwLock<FxHashMap<Font, Result<FontId>>>,
     font_metrics: RwLock<FxHashMap<FontId, FontMetrics>>,
     raster_bounds: RwLock<FxHashMap<RenderGlyphParams, Bounds<DevicePixels>>>,
     wrapper_pool: Mutex<FxHashMap<FontIdWithSize, Vec<LineWrapper>>>,
@@ -91,13 +91,26 @@ impl TextSystem {
 
     /// Get the FontId for the configure font family and style.
     pub fn font_id(&self, font: &Font) -> Result<FontId> {
-        let font_id = self.font_ids_by_font.read().get(font).copied();
+        fn clone_font_id_result(font_id: &Result<FontId>) -> Result<FontId> {
+            match font_id {
+                Ok(font_id) => Ok(*font_id),
+                Err(err) => Err(anyhow!("{}", err)),
+            }
+        }
+
+        let font_id = self
+            .font_ids_by_font
+            .read()
+            .get(font)
+            .map(clone_font_id_result);
         if let Some(font_id) = font_id {
-            Ok(font_id)
+            font_id
         } else {
-            let font_id = self.platform_text_system.font_id(font)?;
-            self.font_ids_by_font.write().insert(font.clone(), font_id);
-            Ok(font_id)
+            let font_id = self.platform_text_system.font_id(font);
+            self.font_ids_by_font
+                .write()
+                .insert(font.clone(), clone_font_id_result(&font_id));
+            font_id
         }
     }