From 9408d70da0a74cd1a16e844d72c612c605d57412 Mon Sep 17 00:00:00 2001 From: Valentin Klein Date: Fri, 6 Nov 2026 17:19:27 +0100 Subject: [PATCH] Disk cache: stamp footage files for the salt off the UI thread (#3106) * Disk cache: stamp footage files for the salt off the UI thread Importing an After Effects project whose footage includes long image sequences on a network share froze the desktop app for 15-27 s. The import itself takes about 0.25 s; the time went into the edit's bump(), where refresh_disk_salt() stats every footage file (13,466 sequence frames in the measured project, ~15 s) on the UI thread whenever a disk cache is attached. File > Open and footage reload hit the same path. With Session::disk_salt_background (set by the desktop app) the salt is taken on a worker thread and picked up by poll_disk_salt() from the UI tick. Until it lands the layer cache runs memory-only, so keys under the old salt (other files, or another project's items) are never read. A newer footage change supersedes a salt still being taken. Headless sessions keep taking it synchronously. A settings change that keeps the disk cache folder no longer re-stamps every footage file either. Co-Authored-By: Claude Opus 5.5 * Disk cache: opening a project drops the footage salt being taken A salt job still running for the old project was kept when the new project named the same footage, so the salt stamped before the files changed stayed in use. Co-Authored-By: Claude Sonnet 5.5 --------- Co-authored-by: Claude Opus 5.5 --- apps/effectcraft/src/main.rs | 3 + crates/engine/src/lib.rs | 10 +- crates/engine/src/session.rs | 2 +- crates/engine/src/session/edit.rs | 98 ++++++++++++++++--- .../engine/src/settings/session_settings.rs | 8 +- crates/engine/src/tests/disk_cache.rs | 98 +++++++++++++++++++ crates/ui-egui/src/app/frame.rs | 4 + 7 files changed, 206 insertions(+), 17 deletions(-) diff --git a/apps/effectcraft/src/main.rs b/apps/effectcraft/src/main.rs index 5350addbe..07d327a21 100644 --- a/apps/effectcraft/src/main.rs +++ b/apps/effectcraft/src/main.rs @@ -276,6 +276,9 @@ fn main() -> eframe::Result { // written on background threads (M13.14). session.check_footage_on_open = true; session.autosave.background = true; + // The disk cache's footage stamps too: an imported or opened project with thousands of + // sequence frames on a network share took half a minute to stamp. + session.disk_salt_background = true; // Settings, shortcut presets and the crash-recovery sentinel live in the platform // config directory. Agent-driven runs (`--control`) skip crash recovery. if let Some(dir) = effectcraft_host::config_dir() { diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index b5250a3e0..e50b15d04 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -97,7 +97,7 @@ pub use error::{EngineError, Result}; pub use event::Event; pub use fonts::{FontRow, font_features, font_menu, font_preview, font_styles, text_families}; pub use services::{FsServices, Importer, Services}; -pub use session::upgrade_effects; +pub use session::{DiskSaltJob, upgrade_effects}; pub struct Session { pub project: Arc, @@ -256,6 +256,12 @@ pub struct Session { pub footage_stamps: std::collections::HashMap, /// The disk cache's footage salt and the footage it was taken for ([`Session::refresh_disk_salt`]). pub disk_salt: Option<(u64, u64)>, + /// Take the disk cache's footage salt on a worker thread ([`Session::refresh_disk_salt`]): + /// frontends with an event loop (the desktop app) turn this on and call + /// [`Session::poll_disk_salt`] each frame; headless sessions take it at once. + pub disk_salt_background: bool, + /// The footage salt being taken in the background. + pub disk_salt_job: Option, /// The footage files effects read the channels of, as last given to the footage source /// ([`Session::refresh_aux_footage`]); `None` = not given yet. pub aux_footage: Option>, @@ -386,6 +392,8 @@ impl Default for Session { stored_paths: Default::default(), footage_stamps: Default::default(), disk_salt: None, + disk_salt_background: false, + disk_salt_job: None, aux_footage: None, script_calls: ScriptCalls::default(), } diff --git a/crates/engine/src/session.rs b/crates/engine/src/session.rs index 18b0ff4cf..31ed40c1d 100644 --- a/crates/engine/src/session.rs +++ b/crates/engine/src/session.rs @@ -10,4 +10,4 @@ mod dispatch; mod edit; mod render; -pub use edit::upgrade_effects; +pub use edit::{DiskSaltJob, upgrade_effects}; diff --git a/crates/engine/src/session/edit.rs b/crates/engine/src/session/edit.rs index ec6a724ac..28813e2ec 100644 --- a/crates/engine/src/session/edit.rs +++ b/crates/engine/src/session/edit.rs @@ -103,29 +103,90 @@ impl Session { } /// Salt the disk cache's layer keys with the project's footage files - /// ([`effectcraft_render::disk_cache::footage_salt`]). Taking it reads the size and time of - /// every file, every frame of an image sequence included, so it is taken again only when the - /// footage items changed, or with `force` (a project opened, footage reloaded, the disk cache - /// attached), not at every edit: with a few large sequences that cost each keystroke in a - /// text field over half a second (#1926). + /// ([`effectcraft_render::disk_cache::footage_salt`]) and attach the disk cache to the layer + /// cache with it. Taking it reads the size and time of every file, every frame of an image + /// sequence included, so it is taken again only when the footage items changed, or with + /// `force` (a project opened, footage reloaded, the disk cache attached), not at every edit: + /// with a few large sequences that cost each keystroke in a text field over half a second + /// (#1926). + /// + /// With [`Session::disk_salt_background`] the files are read on a worker thread: an imported + /// or opened project with thousands of sequence frames on a network share took half a minute + /// to stamp, all of it on the UI thread. Until [`Session::poll_disk_salt`] picks the salt up, + /// renders use the memory cache only (keys under the old salt may name other files, or + /// another project's items) and this returns `None`. pub fn refresh_disk_salt(&mut self, force: bool) -> Option { - if self.disk_cache.is_none() { + let Some(dc) = self.disk_cache.clone() else { self.disk_salt = None; + self.disk_salt_job = None; return None; - } + }; + self.poll_disk_salt(); let footage = Self::footage_fingerprint(&self.project); - if let Some((seen, salt)) = self.disk_salt - && seen == footage - && !force - { - return Some(salt); + if !force { + if let Some((seen, salt)) = self.disk_salt + && seen == footage + { + return Some(salt); + } + if self.disk_salt_job.as_ref().is_some_and(|j| j.footage == footage) { + return None; + } } + if self.disk_salt_background && !cfg!(target_arch = "wasm32") { + let project = self.project.clone(); + let slot: Arc>> = Arc::default(); + let out = slot.clone(); + let spawned = std::thread::Builder::new().name("ec-disk-salt".into()).spawn(move || { + let salt = effectcraft_render::disk_cache::footage_salt(&project); + if let Ok(mut s) = out.lock() { + *s = Some(salt); + } + }); + // Without a thread the salt is taken here, as before. + if let Ok(thread) = spawned { + self.disk_salt = None; + self.disk_salt_job = Some(DiskSaltJob { footage, slot, thread }); + self.layer_cache.set_disk(None); + return None; + } + } + self.disk_salt_job = None; let salt = effectcraft_render::disk_cache::footage_salt(&self.project); self.disk_salt = Some((footage, salt)); - self.layer_cache.set_disk_salt(salt); + self.layer_cache.set_disk(Some((dc, salt))); Some(salt) } + /// Pick up a footage salt taken in the background ([`Session::refresh_disk_salt`]) and attach + /// the disk cache with it. Returns whether one is still being taken (frontends call this each + /// frame while it is). + pub fn poll_disk_salt(&mut self) -> bool { + let Some(job) = &self.disk_salt_job else { return false }; + // (looked at first: a worker that has finished has left its salt in the slot) + let finished = job.thread.is_finished(); + let salt = job.slot.lock().ok().and_then(|mut s| s.take()); + let footage = job.footage; + match salt { + Some(salt) => { + self.disk_salt_job = None; + if let Some(dc) = self.disk_cache.clone() { + self.disk_salt = Some((footage, salt)); + self.layer_cache.set_disk(Some((dc, salt))); + } + false + } + None if finished => { + // The worker stopped without a salt: the disk cache stays off until the next + // edit takes it again. + log::warn!("disk cache: the footage files could not be stamped; the disk cache is off until the next edit"); + self.disk_salt_job = None; + false + } + None => true, + } + } + /// In-memory identity of the footage settings represented by a disk salt. Workers use the /// same fingerprint when preparing availability changes without disk access on the UI. pub(crate) fn footage_fingerprint(project: &effectcraft_project::Project) -> u64 { @@ -263,6 +324,9 @@ impl Session { self.project = Arc::new(p); // Its files may have changed since they were last stamped. self.disk_salt = None; + // A salt still being taken is for the old project's files: dropped, so that the next refresh + // stamps the new project anew (a matching fingerprint would otherwise keep the stale job). + self.disk_salt_job = None; // The set of footage effects read the channels of is sent again, even when it is empty. self.aux_footage = None; self.history = History::default(); @@ -325,3 +389,11 @@ pub fn upgrade_effects(p: &mut Project) { p.next_id = next; p.schema = effectcraft_project::SCHEMA_VERSION; } + +/// A footage salt being taken on a worker thread ([`Session::refresh_disk_salt`]). +pub struct DiskSaltJob { + /// The footage it is taken for ([`Session::footage_fingerprint`]). + footage: u64, + slot: Arc>>, + thread: std::thread::JoinHandle<()>, +} diff --git a/crates/engine/src/settings/session_settings.rs b/crates/engine/src/settings/session_settings.rs index 4fe47e0ef..056616201 100644 --- a/crates/engine/src/settings/session_settings.rs +++ b/crates/engine/src/settings/session_settings.rs @@ -137,6 +137,10 @@ impl Session { // over the store of a roto test running beside it. Serialise them with the roto tests. #[cfg(test)] crate::tests::roto::hold_roto_cache_test_lock(); + // Another setting changed (the disk cache stays as it is): its salt still holds, and + // stamping every footage file again would cost each settings change seconds with large + // sequences. + let kept = self.disk_cache.as_ref().is_some_and(|c| c.folder() == folder.as_path()); let dc = match &self.disk_cache { Some(c) if c.folder() == folder.as_path() => { c.set_max_bytes(max); @@ -161,8 +165,8 @@ impl Session { self.disk_cache = Some(dc.clone()); // The reserve applies from the start (a new folder's watcher is idle). self.poll_disk_space(); - let salt = self.refresh_disk_salt(true).unwrap_or_default(); - self.layer_cache.set_disk(Some((dc.clone(), salt))); + // Attaches the disk cache to the layer cache once the footage salt is known. + self.refresh_disk_salt(!kept); // Roto Brush / Object Matte segmentations persist there too (held weakly: switching the // disk cache off drops it). let store: std::sync::Arc = dc.clone(); diff --git a/crates/engine/src/tests/disk_cache.rs b/crates/engine/src/tests/disk_cache.rs index 41497dd4b..4805d68fe 100644 --- a/crates/engine/src/tests/disk_cache.rs +++ b/crates/engine/src/tests/disk_cache.rs @@ -137,6 +137,68 @@ fn footage_changes_change_the_salt() { assert_ne!(set_proxy(&mut s, false), k3); } +/// The desktop app stamps footage files for the disk cache's salt on a worker thread: importing an +/// After Effects project with thousands of sequence frames on a network share stamped them all +/// inside the import's edit, freezing the UI for half a minute. Until the salt lands the disk +/// cache sits out (keys under the old salt may name other files); a newer footage change +/// supersedes a salt still being taken; a settings change keeps the salt. +#[test] +fn footage_salt_is_taken_off_the_ui_thread_and_the_disk_cache_waits_for_it() { + let dir = folder("bg-salt"); + let mut s = session(&dir); + s.disk_salt_background = true; + let at = |n: &str| std::env::temp_dir().join(format!("effectcraft-bg-salt-{}-{n}.png", std::process::id())); + let add = |s: &mut Session, path: &std::path::Path| { + let f = effectcraft_project::Footage { + path: path.to_string_lossy().to_string(), + kind: effectcraft_project::FootageKind::Still, + width: 1, + height: 1, + has_video: true, + ..Default::default() + }; + s.edit("add", None, |p, _| { + p.add_item("f.png", effectcraft_color::Label::None, None, effectcraft_project::ItemKind::Footage(f)); + Ok(()) + }) + .unwrap(); + }; + let wait = |s: &mut Session| { + let t0 = std::time::Instant::now(); + while s.poll_disk_salt() { + assert!(t0.elapsed().as_secs() < 20, "the salt never landed"); + std::thread::sleep(std::time::Duration::from_millis(5)); + } + }; + assert!(s.layer_cache.disk().is_some(), "attached with its salt"); + std::fs::write(at("a"), b"one").unwrap(); + add(&mut s, &at("a")); + // The edit returned before the files were stamped, and the disk cache waits for the salt. + assert!(s.disk_salt.is_none() && s.layer_cache.disk().is_none()); + // Edits that leave the footage alone don't start over (and may pick up the salt meanwhile). + s.execute("comp.new", json!({"name": "C", "width": 8, "height": 8, "frameRate": 30, "duration": 1})).unwrap(); + let r = s.refresh_disk_salt(false); + assert!(r.is_none() || r == Some(effectcraft_render::disk_cache::footage_salt(&s.project)), "{r:?}"); + // New footage while it is taken: the salt for the newest footage lands. + std::fs::write(at("b"), b"two").unwrap(); + add(&mut s, &at("b")); + wait(&mut s); + let salt = effectcraft_render::disk_cache::footage_salt(&s.project); + assert_eq!(s.disk_salt.map(|(_, salt)| salt), Some(salt)); + assert_eq!(s.refresh_disk_salt(false), Some(salt)); + assert!(s.layer_cache.disk().is_some(), "attached again"); + // Another setting keeps the salt and the disk cache attached. + s.execute_checked("prefs.set", json!({"key": "disk.diskCacheMaxGb", "value": 30})).unwrap(); + assert!(s.layer_cache.disk().is_some() && s.disk_salt.map(|(_, salt)| salt) == Some(salt)); + // Turned off and on again: stamped anew, in the background. + s.execute_checked("prefs.set", json!({"key": "disk.diskCacheEnabled", "value": false})).unwrap(); + s.execute_checked("prefs.set", json!({"key": "disk.diskCacheEnabled", "value": true})).unwrap(); + wait(&mut s); + assert_eq!(s.refresh_disk_salt(false), Some(salt)); + let _ = std::fs::remove_file(at("a")); + let _ = std::fs::remove_file(at("b")); +} + /// Marker edits keep the disk key (the sound of a playing preview must not wait for frames to /// render again, #1083); an expression that reads markers makes them count. #[test] @@ -195,3 +257,39 @@ fn protected_markers_with_a_duration_change_the_content_key() { s.execute("markers.set", json!({"comp": inner.0, "index": index, "duration": 2.0})).unwrap(); assert_ne!(key(&s), k1, "its duration"); } + +/// A project opened while a footage salt is still being taken must not keep that salt when it +/// names the same footage: its files may have changed since the worker read them. +#[test] +fn opening_a_project_drops_the_footage_salt_being_taken() { + let dir = folder("bg-salt-open"); + let mut s = session(&dir); + s.disk_salt_background = true; + let path = std::env::temp_dir().join(format!("effectcraft-bg-salt-open-{}.png", std::process::id())); + std::fs::write(&path, b"one").unwrap(); + let f = effectcraft_project::Footage { + path: path.to_string_lossy().to_string(), + kind: effectcraft_project::FootageKind::Still, + width: 1, + height: 1, + has_video: true, + ..Default::default() + }; + s.edit("add", None, |p, _| { + p.add_item("f.png", effectcraft_color::Label::None, None, effectcraft_project::ItemKind::Footage(f)); + Ok(()) + }) + .unwrap(); + // The file changes after the edit started the salt, before the same footage is opened again. + std::fs::write(&path, b"a longer content").unwrap(); + let same = (*s.project).clone(); + s.replace_project(same, None); + let t0 = std::time::Instant::now(); + while s.poll_disk_salt() { + assert!(t0.elapsed().as_secs() < 20, "the salt never landed"); + std::thread::sleep(std::time::Duration::from_millis(5)); + } + let now = effectcraft_render::disk_cache::footage_salt(&s.project); + assert_eq!(s.disk_salt.map(|(_, salt)| salt), Some(now), "the salt reflects the file as it is now"); + let _ = std::fs::remove_file(&path); +} diff --git a/crates/ui-egui/src/app/frame.rs b/crates/ui-egui/src/app/frame.rs index 8ebea08fc..e77f07840 100644 --- a/crates/ui-egui/src/app/frame.rs +++ b/crates/ui-egui/src/app/frame.rs @@ -170,6 +170,10 @@ impl EffectcraftApp { self.session.poll_jobs(); ctx.request_repaint_after(std::time::Duration::from_millis(100)); } + // The disk cache's footage salt, taken off the UI thread after footage changed. + if self.session.poll_disk_salt() { + ctx.request_repaint_after(std::time::Duration::from_millis(100)); + } // Analyses running in a worker (the web app). if !self.session.offloaded.is_empty() { self.session.poll_offload();