Merge pull request #2348 from vangel76/effects-cache-analysis-keys

Reuse Warp Stabilizer and Camera Tracker analysis keys

crates/effects/src/analysis_cache.rs

@@ -0,0 +1,97 @@
+//! Bounded parsed-JSON caches. Keep the content key with the owned source text so repeated
+//! renders do not rehash long analyses. Compare contents on lookup: evaluated parameter
+//! strings can be cloned, edited in place, or allocated again at an old address.
+
+use std::sync::{Arc, Mutex, OnceLock};
+
+struct Entry<T> {
+    json: Box<str>,
+    key: u64,
+    value: Arc<T>,
+}
+
+pub(crate) struct AnalysisCache<T> {
+    entries: OnceLock<Mutex<Vec<Entry<T>>>>,
+}
+
+pub(crate) fn fnv(bytes: &[u8]) -> u64 {
+    let mut h: u64 = 0xcbf2_9ce4_8422_2325;
+    for b in bytes {
+        h ^= *b as u64;
+        h = h.wrapping_mul(0x0100_0000_01b3);
+    }
+    h
+}
+
+impl<T> AnalysisCache<T> {
+    pub(crate) const fn new() -> Self {
+        Self { entries: OnceLock::new() }
+    }
+
+    pub(crate) fn get(&self, json: &str, parse: impl FnOnce(&str) -> Option<T>) -> Option<(u64, Arc<T>)> {
+        let entries = self.entries.get_or_init(Default::default);
+        {
+            let m = entries.lock().unwrap_or_else(|e| e.into_inner());
+            if let Some(e) = m.iter().rev().find(|e| e.json.as_ref() == json) {
+                return Some((e.key, e.value.clone()));
+            }
+        }
+        let value = Arc::new(parse(json)?);
+        let key = fnv(json.as_bytes());
+        let mut m = entries.lock().unwrap_or_else(|e| e.into_inner());
+        // Another render may have filled the entry while this one parsed it.
+        if let Some(e) = m.iter().rev().find(|e| e.json.as_ref() == json) {
+            return Some((e.key, e.value.clone()));
+        }
+        // Preserve the existing nine-entry bound and clear-on-overflow policy.
+        if m.len() > 8 {
+            m.clear();
+        }
+        m.push(Entry { json: json.into(), key, value: value.clone() });
+        Some((key, value))
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use std::cell::Cell;
+
+    #[test]
+    fn reuses_content_and_invalidates_same_allocation_edits() {
+        let cache = AnalysisCache::new();
+        let calls = Cell::new(0);
+        let parse = |s: &str| {
+            calls.set(calls.get() + 1);
+            Some(s.to_owned())
+        };
+        let mut json = String::from("analysis-1");
+        let (key, a) = cache.get(&json, parse).unwrap();
+        let copy = json.clone();
+        assert_ne!(json.as_ptr(), copy.as_ptr());
+        let (same_key, same) = cache.get(&copy, parse).unwrap();
+        assert_eq!(key, fnv(json.as_bytes()));
+        assert_eq!(key, same_key);
+        assert!(Arc::ptr_eq(&a, &same));
+        assert_eq!(calls.get(), 1);
+        let ptr = json.as_ptr();
+        json.replace_range(9..10, "2");
+        assert_eq!(ptr, json.as_ptr());
+        let (changed_key, changed) = cache.get(&json, parse).unwrap();
+        assert_ne!(key, changed_key);
+        assert_eq!(&*changed, "analysis-2");
+        assert_eq!(&*a, "analysis-1");
+        assert_eq!(calls.get(), 2);
+        assert!(cache.get("invalid", |_| None).is_none());
+        assert!(Arc::ptr_eq(&cache.get(&json, parse).unwrap().1, &changed));
+    }
+
+    #[test]
+    fn source_text_retention_is_bounded() {
+        let cache = AnalysisCache::new();
+        for i in 0..30 {
+            cache.get(&i.to_string(), |s| Some(s.to_owned())).unwrap();
+            assert!(cache.entries.get().unwrap().lock().unwrap().len() <= 9);
+        }
+    }
+}

crates/effects/src/camera_tracker.rs

@@ -16,8 +16,9 @@
 //! on it draws the solved points (as the viewer shows them) into the frame. The viewer overlay, target and the Create
 //! commands live in the engine and the UI.
 
-use std::collections::HashMap;
-use std::sync::{Arc, Mutex, OnceLock};
+use std::sync::Arc;
+
+use crate::analysis_cache::AnalysisCache;
 
 use effectcraft_keyframe::Value;
 use effectcraft_project::ParamUi;
@@ -122,50 +123,24 @@ pub fn solve_key(st: &SolveSettings) -> String {
     format!("{:?}|{hfov}|{:?}|{:?}{lens}", st.shot, st.method, st.deleted)
 }
 
-fn fnv(bytes: &[u8]) -> u64 {
-    let mut h: u64 = 0xcbf2_9ce4_8422_2325;
-    for b in bytes {
-        h ^= *b as u64;
-        h = h.wrapping_mul(0x0100_0000_01b3);
-    }
-    h
-}
-
-type Cache<T> = Mutex<HashMap<u64, Arc<T>>>;
-
-fn cached<T>(c: &'static OnceLock<Cache<T>>, json: &str, parse: impl Fn(&str) -> Option<T>) -> Option<Arc<T>> {
-    let key = fnv(json.as_bytes());
-    let c = c.get_or_init(Default::default);
-    if let Some(a) = c.lock().unwrap_or_else(std::sync::PoisonError::into_inner).get(&key) {
-        return Some(a.clone());
-    }
-    let a = Arc::new(parse(json)?);
-    let mut m = c.lock().unwrap_or_else(std::sync::PoisonError::into_inner);
-    if m.len() > 8 {
-        m.clear();
-    }
-    m.insert(key, a.clone());
-    Some(a)
-}
-
 /// The stored feature tracks of an instance (`None` when not analysed).
 pub fn tracks(params: &Params) -> Option<Arc<CameraTracks>> {
-    static C: OnceLock<Cache<CameraTracks>> = OnceLock::new();
+    static C: AnalysisCache<CameraTracks> = AnalysisCache::new();
     let j = s(params, TRACKS);
     if j.is_empty() {
         return None;
     }
-    cached(&C, j, CameraTracks::from_json).filter(|t| !t.is_empty())
+    C.get(j, CameraTracks::from_json).map(|(_, a)| a).filter(|t| !t.is_empty())
 }
 
 /// The stored camera solve of an instance (`None` when not solved).
 pub fn solve(params: &Params) -> Option<Arc<CameraSolve>> {
-    static C: OnceLock<Cache<CameraSolve>> = OnceLock::new();
+    static C: AnalysisCache<CameraSolve> = AnalysisCache::new();
     let j = s(params, SOLVE);
     if j.is_empty() {
         return None;
     }
-    cached(&C, j, CameraSolve::from_json).filter(|t| !t.is_empty())
+    C.get(j, CameraSolve::from_json).map(|(_, a)| a).filter(|t| !t.is_empty())
 }
 
 /// Screen size (layer pixels) of a target drawn for a point at depth `z`, relative to the
@@ -338,6 +313,27 @@ mod tests {
         assert!(!st.lens_distortion && !solve_key(&st).contains("lens"));
     }
 
+    #[test]
+    fn tracks_and_solve_follow_analysis_replacements() {
+        let mut t = CameraTracks { frames: 3, start: 1.0, ..Default::default() };
+        let mut sol = CameraSolve { frames: vec![SolvedFrame::default()], start: 1.0, ..Default::default() };
+        let mut ps = params_with(&sol, false);
+        ps.values.insert(TRACKS.into(), Value::Str(t.to_json()));
+        assert_eq!(*tracks(&ps).unwrap(), t);
+        assert_eq!(*solve(&ps).unwrap(), sol);
+        t.start = 2.0;
+        sol.start = 2.0;
+        ps.values.insert(TRACKS.into(), Value::Str(t.to_json()));
+        ps.values.insert(SOLVE.into(), Value::Str(sol.to_json()));
+        assert_eq!(*tracks(&ps).unwrap(), t);
+        assert_eq!(*solve(&ps).unwrap(), sol);
+        assert_eq!(*tracks(&ps.clone()).unwrap(), t);
+        ps.values.insert(TRACKS.into(), Value::Str("invalid".into()));
+        ps.values.insert(SOLVE.into(), Value::Str(String::new()));
+        assert!(tracks(&ps).is_none());
+        assert!(solve(&ps).is_none());
+    }
+
     #[test]
     fn undistort_footage_remaps_through_the_solved_lens() {
         use effectcraft_track::camtrack::Distortion;

crates/effects/src/lib.rs

@@ -8,6 +8,7 @@
 
 #![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)]
 
+mod analysis_cache;
 pub mod audio_fx;
 mod blur2;
 mod blur3;

crates/effects/src/warp_stab.rs

@@ -152,15 +152,6 @@ pub fn settings(params: &Params, fps: f64) -> StabSettings {
     }
 }
 
-fn fnv(bytes: &[u8]) -> u64 {
-    let mut h: u64 = 0xcbf2_9ce4_8422_2325;
-    for b in bytes {
-        h ^= *b as u64;
-        h = h.wrapping_mul(0x0100_0000_01b3);
-    }
-    h
-}
-
 struct KeyHasher(u64);
 impl Hasher for KeyHasher {
     fn finish(&self) -> u64 {
@@ -183,24 +174,12 @@ fn settings_key(s: &StabSettings) -> u64 {
     h.finish()
 }
 
-type AnalysisCache = Mutex<HashMap<u64, Arc<WarpAnalysis>>>;
 type PlanCache = Mutex<HashMap<(u64, u64), Arc<Plan>>>;
 
-/// Parsed analyses by content hash (parsing a long clip's JSON every frame would dominate).
+/// Parsed analyses and their content keys; cache hits compare source text without rehashing.
 fn analysis_cached(json: &str) -> Option<(u64, Arc<WarpAnalysis>)> {
-    static C: OnceLock<AnalysisCache> = OnceLock::new();
-    let key = fnv(json.as_bytes());
-    let c = C.get_or_init(Default::default);
-    if let Some(a) = c.lock().unwrap_or_else(std::sync::PoisonError::into_inner).get(&key) {
-        return Some((key, a.clone()));
-    }
-    let a = Arc::new(WarpAnalysis::from_json(json)?);
-    let mut m = c.lock().unwrap_or_else(std::sync::PoisonError::into_inner);
-    if m.len() > 8 {
-        m.clear();
-    }
-    m.insert(key, a.clone());
-    Some((key, a))
+    static C: crate::analysis_cache::AnalysisCache<WarpAnalysis> = crate::analysis_cache::AnalysisCache::new();
+    C.get(json, WarpAnalysis::from_json)
 }
 
 fn plans() -> &'static PlanCache {
@@ -544,6 +523,7 @@ fn dot(img: &mut Image, c: [f64; 2], r: f64, col: Px) {
 mod tests {
     use super::*;
     use crate::EffectEnv;
+    use crate::analysis_cache::fnv;
     use effectcraft_track::stabilize::FrameMotion;
 
     fn analysis_json(n: usize, w: f64, h: f64, shift: impl Fn(usize) -> [f64; 2]) -> String {
@@ -650,6 +630,25 @@ mod tests {
         assert!(!prepare_default_plan(&WarpAnalysis::default().to_json()));
     }
 
+    #[test]
+    fn changing_analysis_changes_the_plan_key() {
+        let json = analysis_json(10, 64.0, 48.0, |k| [k as f64 * 0.1, 0.0]);
+        let mut ps = params(&json, 2, 0);
+        let before = summary_key(&ps).unwrap();
+        assert_eq!(before[0], fnv(json.as_bytes()));
+        let mut a = WarpAnalysis::from_json(&json).unwrap();
+        a.start = 1.0;
+        ps.values.insert(ANALYSIS.into(), Value::Str(a.to_json()));
+        let after = summary_key(&ps).unwrap();
+        assert_ne!(before[0], after[0]);
+        assert_eq!(before[1], after[1]);
+        assert_eq!(*analysis(&ps).unwrap(), a);
+        ps.values.insert(ANALYSIS.into(), Value::Str(json));
+        assert_eq!(summary_key(&ps).unwrap(), before);
+        ps.values.insert(ANALYSIS.into(), Value::Str(String::new()));
+        assert!(analysis(&ps).is_none());
+    }
+
     #[test]
     fn nested_param_lookup() {
         let mut p = Params::default();