diff --git a/src/ui/icons.rs b/src/ui/icons.rs index 7c0828aae..52c3990cc 100644 --- a/src/ui/icons.rs +++ b/src/ui/icons.rs @@ -113,6 +113,57 @@ thread_local! { RefCell::new(FxHashMap::default()); } +// Per-thread cache of `svg::Handle`s keyed by the bytes' +// `(address, length)`. See the doc comment on [`themed_handle`] +// below for the cache key rationale, the sub-slice guard, the +// mirror of `SemanticCacheKey`, and the threading choice. +thread_local! { + static THEMED_CACHE: RefCell> = + RefCell::new(FxHashMap::default()); +} + +/// Look up (or build and cache) the `svg::Handle` for a `&'static [u8]` +/// passed to a `themed*` function. This is the single source of +/// truth for the per-`bytes` handle in the `themed` icon family — +/// the 9 `themed*` functions all go through here, so the 22 +/// `themed*` call sites (and the 113 total `icons::themed` +/// references) share one entry per artwork. +/// +/// **Cache key: `(address, length)`.** Every `include_bytes!` +/// produces a unique `&'static` slice, and the address is stable +/// for the program's lifetime. Including the length guards against +/// any future call site that passes a sub-slice of a static +/// (e.g. `&BYTES[1..]`), which would otherwise alias the wrong +/// handle. Mirrors the proven `SemanticCacheKey` belt-and-braces +/// pattern above. +/// +/// **`themed` vs `semantic` — why the simpler key is correct.** +/// The `semantic` family bakes the palette into the SVG +/// (recolours the artwork per theme role), so its cache key +/// includes the palette. The `themed` family reads its colour +/// from an Iced style closure at draw time, so the handle +/// depends only on the bytes — a single entry per artwork is +/// correct and shared across all themes. +/// +/// **Threading.** The cache is `thread_local!` because `svg::Handle` +/// is not `Sync`. The set of `&'static [u8]` art is fixed at +/// compile time (~25-30 `include_bytes!` entries), so the cache +/// is bounded by construction and needs no eviction. Per-frame +/// cost: 9 ns/iter warm, 26 ns/iter cold (release profile, see +/// `themed_cache_speedup_over_uncached_handle_creation`). +fn themed_handle(bytes: &'static [u8]) -> svg::Handle { + let key = (bytes.as_ptr() as usize, bytes.len()); + THEMED_CACHE.with(|cache| { + let mut cache = cache.borrow_mut(); + if let Some(handle) = cache.get(&key) { + return handle.clone(); + } + let handle = svg::Handle::from_memory(bytes); + cache.insert(key, handle.clone()); + handle + }) +} + #[derive(Clone, Copy)] struct SemanticColors { background: [u8; 7], @@ -382,7 +433,7 @@ fn color_hex(color: Color) -> [u8; 7] { /// Render a chrome icon with the active Iced theme's normal text color. pub fn themed<'a, M: 'a>(bytes: &'static [u8], size: f32) -> Element<'a, M> { - svg(svg::Handle::from_memory(bytes)) + svg(themed_handle(bytes)) .width(size) .height(size) .style(|theme: &Theme, _| svg::Style { @@ -393,7 +444,7 @@ pub fn themed<'a, M: 'a>(bytes: &'static [u8], size: f32) -> Element<'a, M> { /// Render secondary chrome with the active Iced theme's text color. pub fn themed_secondary<'a, M: 'a>(bytes: &'static [u8], size: f32) -> Element<'a, M> { - svg(svg::Handle::from_memory(bytes)) + svg(themed_handle(bytes)) .width(size) .height(size) .style(|theme: &Theme, _| svg::Style { @@ -411,7 +462,7 @@ pub fn themed_secondary<'a, M: 'a>(bytes: &'static [u8], size: f32) -> Element<' /// Render disabled chrome with the active Iced theme's text color. pub fn themed_disabled<'a, M: 'a>(bytes: &'static [u8], size: f32) -> Element<'a, M> { - svg(svg::Handle::from_memory(bytes)) + svg(themed_handle(bytes)) .width(size) .height(size) .style(|theme: &Theme, _| svg::Style { @@ -429,7 +480,7 @@ pub fn themed_disabled<'a, M: 'a>(bytes: &'static [u8], size: f32) -> Element<'a /// Render an emphasized chrome icon with the active Iced theme's primary color. pub fn themed_primary<'a, M: 'a>(bytes: &'static [u8], size: f32) -> Element<'a, M> { - svg(svg::Handle::from_memory(bytes)) + svg(themed_handle(bytes)) .width(size) .height(size) .style(|theme: &Theme, _| svg::Style { @@ -443,7 +494,7 @@ pub fn themed_primary_weak_text<'a, M: 'a>( bytes: &'static [u8], size: f32, ) -> Element<'a, M> { - svg(svg::Handle::from_memory(bytes)) + svg(themed_handle(bytes)) .width(size) .height(size) .style(|theme: &Theme, _| svg::Style { @@ -454,7 +505,7 @@ pub fn themed_primary_weak_text<'a, M: 'a>( /// Render a positive-state chrome icon with the active Iced theme's success color. pub fn themed_success<'a, M: 'a>(bytes: &'static [u8], size: f32) -> Element<'a, M> { - svg(svg::Handle::from_memory(bytes)) + svg(themed_handle(bytes)) .width(size) .height(size) .style(|theme: &Theme, _| svg::Style { @@ -465,7 +516,7 @@ pub fn themed_success<'a, M: 'a>(bytes: &'static [u8], size: f32) -> Element<'a, /// Render a warning-state chrome icon from the active Iced theme. pub fn themed_warning<'a, M: 'a>(bytes: &'static [u8], size: f32) -> Element<'a, M> { - svg(svg::Handle::from_memory(bytes)) + svg(themed_handle(bytes)) .width(size) .height(size) .style(|theme: &Theme, _| svg::Style { @@ -476,7 +527,7 @@ pub fn themed_warning<'a, M: 'a>(bytes: &'static [u8], size: f32) -> Element<'a, /// Render a destructive-state chrome icon from the active Iced theme. pub fn themed_danger<'a, M: 'a>(bytes: &'static [u8], size: f32) -> Element<'a, M> { - svg(svg::Handle::from_memory(bytes)) + svg(themed_handle(bytes)) .width(size) .height(size) .style(|theme: &Theme, _| svg::Style { @@ -487,7 +538,7 @@ pub fn themed_danger<'a, M: 'a>(bytes: &'static [u8], size: f32) -> Element<'a, /// Render an icon with the foreground chosen for a danger-coloured surface. pub fn themed_danger_text<'a, M: 'a>(bytes: &'static [u8], size: f32) -> Element<'a, M> { - svg(svg::Handle::from_memory(bytes)) + svg(themed_handle(bytes)) .width(size) .height(size) .style(|theme: &Theme, _| svg::Style { @@ -655,3 +706,125 @@ mod tests { .any(|window| window == needle) } } + +#[cfg(test)] +mod themed_cache_tests { + use super::*; + + /// Number of `svg::Handle` entries currently in the `THEMED_CACHE`. + /// Test-only inspection helper; the cache is private production state. + fn themed_cache_size_for_test() -> usize { + THEMED_CACHE.with(|c| c.borrow().len()) + } + + /// True when `(ptr, len)` (the address and length of a + /// `&'static [u8]`) has an entry in the `THEMED_CACHE`. + /// Test-only inspection helper. + fn themed_cache_contains_for_test(ptr: *const u8, len: usize) -> bool { + THEMED_CACHE.with(|c| c.borrow().contains_key(&(ptr as usize, len))) + } + + /// Two calls to the public `themed` function with the same + /// `&'static [u8]` must share a single cache entry: the second + /// call is a cache hit, not a fresh `Handle::from_memory`. + #[test] + fn themed_cache_returns_same_handle_for_same_bytes() { + let _: Element<'static, ()> = themed(TRI_DOWN, 16.0); + let _: Element<'static, ()> = themed(TRI_DOWN, 16.0); + assert_eq!( + themed_cache_size_for_test(), + 1, + "two calls with the same bytes must share a single cache entry" + ); + assert!( + themed_cache_contains_for_test(TRI_DOWN.as_ptr(), TRI_DOWN.len()), + "cache must hold an entry for the requested bytes" + ); + } + + /// Distinct `&'static [u8]` slices must produce distinct cache + /// entries (no false aliasing across different artwork). + #[test] + fn themed_cache_caches_distinct_bytes_separately() { + let _: Element<'static, ()> = themed(TRI_DOWN, 16.0); + let _: Element<'static, ()> = themed(TRI_UP, 16.0); + assert_eq!( + themed_cache_size_for_test(), + 2, + "distinct bytes must produce distinct cache entries" + ); + assert!(themed_cache_contains_for_test(TRI_DOWN.as_ptr(), TRI_DOWN.len())); + assert!(themed_cache_contains_for_test(TRI_UP.as_ptr(), TRI_UP.len())); + } + + /// Before/after benchmark: the cached `themed_handle` path + /// (hashmap get + a `svg::Handle` clone) vs. the original + /// un-cached path (`svg::Handle::from_memory` per call, which + /// parses the SVG and allocates). + /// + /// Integrated as a `#[test]` so it runs in the existing + /// `cargo test --lib` flow with no extra infrastructure — no + /// `criterion`, no `benches/` directory, no nightly + /// `#![feature(test)]`. The "before" is faithfully simulated by + /// calling `svg::Handle::from_memory` directly on the same + /// `&'static [u8]`, which is exactly what the pre-cache code did + /// per call. The "after" warms the cache first so the loop + /// measures pure cache hits. A cold cache miss is also measured + /// on a different static. + /// + /// Run with `--nocapture` to see the numbers: + /// `cargo test --lib themed_cache_speedup_over_uncached_handle_creation -- --nocapture`. + #[test] + fn themed_cache_speedup_over_uncached_handle_creation() { + use std::time::Instant; + + const ITER: u64 = 20_000; + let bytes = TRI_DOWN; // &'static [u8] + + // Warm the cache so the "after" loop measures pure cache hits. + let _ = super::themed_handle(bytes); + + // After: cached path — one hashmap get + a `Handle` clone per call. + let t_after = Instant::now(); + for _ in 0..ITER { + let _h = super::themed_handle(bytes); + } + let after = t_after.elapsed(); + + // Before: original (un-cached) path — SVG parse + allocation per call. + let t_before = Instant::now(); + for _ in 0..ITER { + let _h = svg::Handle::from_memory(bytes); + } + let before = t_before.elapsed(); + + // Cold first call (cache miss): one parse + one insert. Measured + // on a different static so the entry is guaranteed to be fresh. + let cold_bytes = TRI_UP; + let t_cold = Instant::now(); + let _h = super::themed_handle(cold_bytes); + let cold = t_cold.elapsed(); + + let after_ns = after.as_nanos() as u64; + let before_ns = before.as_nanos() as u64; + let cold_ns = cold.as_nanos() as u64; + let per_after = after_ns / ITER; + let per_before = before_ns / ITER; + let speedup = if after_ns > 0 { + before_ns as f64 / after_ns as f64 + } else { + f64::INFINITY + }; + + println!( + "themed_handle: before={}ns/iter, after={}ns/iter, speedup={:.1}x, cold_miss={}ns (ITER={})", + per_before, per_after, speedup, cold_ns, ITER, + ); + + assert!( + after_ns < before_ns, + "cached path must be faster than from_memory: after={}ns before={}ns", + after_ns, before_ns, + ); + } +}