From 66a94f3ca14708606b01251e60f25df8ff3568f9 Mon Sep 17 00:00:00 2001 From: vangel76 Date: Sun, 25 Oct 2026 00:22:46 +0200 Subject: [PATCH] Reuse analysis content keys across effect renders Keep parsed analyses, their FNV key and source text in a bounded shared cache. Compare source contents on hits instead of rehashing the JSON, so cloned parameter strings remain fast while in-place edits invalidate the cache. Cover analysis replacement, key equality, same-allocation edits and retention bounds. Part of #2322 --- crates/effects/src/analysis_cache.rs | 97 ++++++++++++++++++++++++++++ crates/effects/src/camera_tracker.rs | 60 ++++++++--------- crates/effects/src/lib.rs | 1 + crates/effects/src/warp_stab.rs | 47 +++++++------- 4 files changed, 149 insertions(+), 56 deletions(-) create mode 100644 crates/effects/src/analysis_cache.rs diff --git a/crates/effects/src/analysis_cache.rs b/crates/effects/src/analysis_cache.rs new file mode 100644 index 000000000..98764d205 --- /dev/null +++ b/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 { + json: Box, + key: u64, + value: Arc, +} + +pub(crate) struct AnalysisCache { + entries: OnceLock>>>, +} + +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 AnalysisCache { + pub(crate) const fn new() -> Self { + Self { entries: OnceLock::new() } + } + + pub(crate) fn get(&self, json: &str, parse: impl FnOnce(&str) -> Option) -> Option<(u64, Arc)> { + 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(©, 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); + } + } +} diff --git a/crates/effects/src/camera_tracker.rs b/crates/effects/src/camera_tracker.rs index ff4e51fe4..c58df3593 100644 --- a/crates/effects/src/camera_tracker.rs +++ b/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 = Mutex>>; - -fn cached(c: &'static OnceLock>, json: &str, parse: impl Fn(&str) -> Option) -> Option> { - let key = fnv(json.as_bytes()); - let c = c.get_or_init(Default::default); - if let Some(a) = c.lock().unwrap_or_else(|e| e.into_inner()).get(&key) { - return Some(a.clone()); - } - let a = Arc::new(parse(json)?); - let mut m = c.lock().unwrap_or_else(|e| e.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> { - static C: OnceLock> = OnceLock::new(); + static C: AnalysisCache = 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> { - static C: OnceLock> = OnceLock::new(); + static C: AnalysisCache = 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; diff --git a/crates/effects/src/lib.rs b/crates/effects/src/lib.rs index cc0aad515..1111a4e0a 100644 --- a/crates/effects/src/lib.rs +++ b/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; diff --git a/crates/effects/src/warp_stab.rs b/crates/effects/src/warp_stab.rs index 595cb5200..90a414f47 100644 --- a/crates/effects/src/warp_stab.rs +++ b/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>>; type PlanCache = Mutex>>; -/// 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)> { - static C: OnceLock = 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(|e| e.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(|e| e.into_inner()); - if m.len() > 8 { - m.clear(); - } - m.insert(key, a.clone()); - Some((key, a)) + static C: crate::analysis_cache::AnalysisCache = crate::analysis_cache::AnalysisCache::new(); + C.get(json, WarpAnalysis::from_json) } fn plans() -> &'static PlanCache { @@ -534,6 +513,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 { @@ -604,6 +584,25 @@ mod tests { assert_eq!(render(&ctx, Buf { img: img.clone(), offset: [0.0; 2], scale: 1.0 }).img.data, img.data); } + #[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();