From f4cd97471cd5a52b62dbc9aa1b72814a598d837b Mon Sep 17 00:00:00 2001
From: Fred Clausen <43556888+fredclausen@users.noreply.github.com>
Date: Mon, 27 Jul 2026 15:01:00 -0600
Subject: [PATCH] perf: FaceId-keyed FontManager caches -> FxHashMap (issue
 #459 item 1)

Switches fallback_metrics_cache, face_cache, and plan_cache from the
default SipHash-based HashMap to FxHashMap, matching the pattern
already used by atlas.rs's glyph cache. FaceId is a small Copy enum
(bare discriminants plus one System(usize) variant); none of the
three maps' keys are influenced by untrusted input, so this is a
safe, mechanical hasher swap.

Deliberately left unchanged: glyph_cache and system_fallback_cache,
both keyed (in whole or part) by char, which is derived from PTY
output. Note the existing atlas.rs procedural_entries map is already
char-keyed FxHashMap, so the codebase's HashDoS posture on char keys
isn't actually uniform today -- this change doesn't attempt to
reconcile that, it only avoids introducing a *new* untrusted-input
case beyond what already exists.

plan_cache's PlanKey also carries Script/Direction derived from
guess_segment_properties() over terminal-run text -- these are
input-influenced, not free of it, but their realizable domain is a
small, fixed, enumerable set of ISO-15924 script tags plus a 5-variant
direction enum, so the conclusion (safe for FxHash) holds even though
the input-derivation itself is real.

Validation:
- freminal-buffer/benches shaping_ligatures (before/after
  --save-baseline): no statistically significant change -- expected,
  its per-iteration call frequency to face_cache/plan_cache is much
  lower than the sustained multi-row churn that surfaced this cost.
- Real-workload before/after flamegraph (btop full-screen redraw,
  same perf methodology as issue #459): sip::Hasher::write and
  hash_one::<&FaceId>, ~9.00% + ~2.92% self-time in the original
  capture (two of the hottest leaves), are absent from the fixed
  capture with no percent-limit filter; only a 0.04% residual
  attributable to the deliberately-untouched glyph_cache remains.

See #459 for the full profiling investigation this follows up on.
---
 freminal/src/gui/font_manager.rs | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/freminal/src/gui/font_manager.rs b/freminal/src/gui/font_manager.rs
index 39bd547c..ba931c2e 100644
--- a/freminal/src/gui/font_manager.rs
+++ b/freminal/src/gui/font_manager.rs
@@ -19,6 +19,7 @@ use conv2::{ConvUtil, ValueFrom};
 use fontdb::Database;
 use freminal_common::buffer_states::fonts::{FontDecorationFlags, FontDecorations, FontWeight};
 use freminal_common::config::Config;
+use rustc_hash::FxHashMap;
 
 // ---------------------------------------------------------------------------
 //  Bundled font data (compiled into the binary via include_bytes!)
@@ -445,7 +446,7 @@ pub struct FontManager {
     /// `glyph_cache`. `None` means the face has no measurable metrics (or is a
     /// primary face). Interior mutability so the renderer can read metrics
     /// through a shared `&FontManager` while still caching.
-    fallback_metrics_cache: RefCell<HashMap<FaceId, Option<FallbackCellMetrics>>>,
+    fallback_metrics_cache: RefCell<FxHashMap<FaceId, Option<FallbackCellMetrics>>>,
 
     /// Per-face cache of parsed `rustybuzz::Face` instances (Task #430).
     /// Populated lazily by [`Self::build_cached_face`] (invoked from
@@ -454,14 +455,14 @@ pub struct FontManager {
     /// size/DPI change (`set_font_size`, `update_pixels_per_point`), even
     /// though the bytes themselves are unaffected by those. `None` means the
     /// face is not loaded or failed to parse as a rustybuzz `Face`.
-    face_cache: RefCell<HashMap<FaceId, Option<CachedFace>>>,
+    face_cache: RefCell<FxHashMap<FaceId, Option<CachedFace>>>,
 
     /// Per-`(face, ligatures, script, direction)` cache of compiled
     /// `rustybuzz::ShapePlan`s (Task #430). Plan compilation is the
     /// second-most expensive part of cold shaping after face parsing;
     /// caching it avoids recompiling an identical plan on every shape call.
     /// Cleared alongside `face_cache`.
-    plan_cache: RefCell<HashMap<PlanKey, Arc<rustybuzz::ShapePlan>>>,
+    plan_cache: RefCell<FxHashMap<PlanKey, Arc<rustybuzz::ShapePlan>>>,
 
     /// Authoritative cell width in integer pixels.
     cell_width: u32,
@@ -578,9 +579,9 @@ impl FontManager {
             system_fallback_cache: HashMap::new(),
             system_faces: Vec::new(),
             glyph_cache: HashMap::new(),
-            fallback_metrics_cache: RefCell::new(HashMap::new()),
-            face_cache: RefCell::new(HashMap::new()),
-            plan_cache: RefCell::new(HashMap::new()),
+            fallback_metrics_cache: RefCell::new(FxHashMap::default()),
+            face_cache: RefCell::new(FxHashMap::default()),
+            plan_cache: RefCell::new(FxHashMap::default()),
             cell_width,
             cell_height,
             ascent,