diff --git a/crates/compose/src/cache.rs b/crates/compose/src/cache.rs index 5e4cb0911..9cf96e497 100644 --- a/crates/compose/src/cache.rs +++ b/crates/compose/src/cache.rs @@ -62,6 +62,9 @@ pub struct Cache { /// The composition each story (page-independent key) had before its latest recomposition, /// with the story revision it was composed from (repaint only the frames an edit changed). previous: Mutex, Arc)>>, + /// A cache this one reads without writing to: a composition or index found there for the same + /// inputs is used, and anything composed is kept here only. + base: Option>, } /// A signature's hash, standing in for it in another story's signature. @@ -104,6 +107,18 @@ impl Cache { Self::default() } + /// An empty cache that also uses what `base` holds for the same inputs, and never changes + /// `base`. A reader of an older state of the document composes that state here instead of + /// replacing the compositions of the current state in `base`. + pub fn over(base: Arc) -> Self { + Cache { base: Some(base), ..Self::default() } + } + + /// The cache given to [`Cache::over`]. + pub fn base(&self) -> Option<&Arc> { + self.base.as_ref() + } + /// The composed story, from cache or freshly composed. `page` is the absolute page a /// parent-page item is shown on (its page number markers and variables are that page's), or /// `None` for a story's own frames. Pages are identified by index, not name: names repeat when @@ -255,6 +270,14 @@ impl Cache { return e.out.clone(); } } + if let Some(base) = &self.base { + let g = base.map.lock().unwrap_or_else(PoisonError::into_inner); + if let Some(e) = g.0.get(&key) + && e.sig == sig + { + return e.out.clone(); + } + } // Parent-page items are composed per page: markers show its name, variables need its index. let page = page.filter(|_| with_vars); let running = running.then(|| self.running_index(doc)); @@ -368,6 +391,12 @@ impl Cache { { return r.clone(); } + if let Some(base) = &self.base + && let Some((s, r)) = &*base.xrefs.lock().unwrap_or_else(PoisonError::into_inner) + && *s == sig + { + return r.clone(); + } let r = Arc::new(crate::xref::XrefIndex::build(doc, &|sid| self.get(doc, sid, None))); *self.xrefs.lock().unwrap_or_else(PoisonError::into_inner) = Some((sig, r.clone())); r @@ -381,6 +410,12 @@ impl Cache { { return r.clone(); } + if let Some(base) = &self.base + && let Some((s, r)) = &*base.running.lock().unwrap_or_else(PoisonError::into_inner) + && *s == sig + { + return r.clone(); + } let r = Arc::new(RunningIndex::build(doc, &|sid| self.get(doc, sid, None))); *self.running.lock().unwrap_or_else(PoisonError::into_inner) = Some((sig, r.clone())); r diff --git a/crates/compose/src/tests.rs b/crates/compose/src/tests.rs index f15f1e33d..900bced97 100644 --- a/crates/compose/src/tests.rs +++ b/crates/compose/src/tests.rs @@ -8156,6 +8156,25 @@ fn cache_recomposes_when_the_baseline_grid_changes() { assert!((la.baseline - lb.baseline).abs() > 1.0, "{} {}", la.baseline, lb.baseline); } +/// A cache over another uses its compositions of the same state, and composes an older state +/// without replacing the other cache's composition of the current one. +#[test] +fn a_cache_over_another_reads_it_without_writing_to_it() { + let (old, sid, _) = doc_with(LOREM, Rect::new(0.0, 0.0, 200.0, 600.0), ParaAttrs::default()); + let mut new = old.clone(); + new.story_mut(sid).unwrap().rev += 1; + let base = Arc::new(Cache::new()); + let current = base.get(&new, sid, None); + let reader = Cache::over(Arc::clone(&base)); + assert!(reader.base().is_some_and(|b| Arc::ptr_eq(b, &base))); + assert!(Arc::ptr_eq(&reader.get(&new, sid, None), ¤t), "the base's composition is used"); + let earlier = reader.get(&old, sid, None); + assert!(!Arc::ptr_eq(&earlier, ¤t)); + assert!(Arc::ptr_eq(&reader.get(&old, sid, None), &earlier), "kept in the reader"); + assert!(Arc::ptr_eq(&base.get(&new, sid, None), ¤t), "the base keeps the current state's composition"); + assert!(base.composed_for(&old, sid).is_none(), "the base holds nothing of the older state"); +} + /// Two single pages 618 pt tall, top margins 36 and 48, each with an overset 11/13 story aligned /// to a baseline grid (start 0, increment 13) and a frame filling its margin box (bottom 582). fn grid_margin_doc(relative_to: designcraft_doc::GridRelative) -> (Document, [StoryId; 2]) { diff --git a/crates/ui-egui/src/preflight_worker.rs b/crates/ui-egui/src/preflight_worker.rs index 6abb67e0c..ddcdffe68 100644 --- a/crates/ui-egui/src/preflight_worker.rs +++ b/crates/ui-egui/src/preflight_worker.rs @@ -40,6 +40,10 @@ const MAX_WORKERS: usize = 4; // this long. #[cfg(not(target_arch = "wasm32"))] const RETRY_AFTER: Duration = Duration::from_secs(30); +// A worker given no job for this long drops its own compositions, which keep the document they +// were made from alive after it closes. +#[cfg(not(target_arch = "wasm32"))] +const FORGET_AFTER: Duration = Duration::from_secs(60); /// A state of a document as a check sees it: which document, which state of it with which /// compositions (a font reload replaces the cache and bumps the revision), and where it is saved @@ -118,7 +122,17 @@ pub(crate) fn issues(app: &mut DesignApp, ctx: &egui::Context) -> Option Option>> { - let (of, doc) = Checked::new(app)?; + let Some((of, doc)) = Checked::new(app) else { + // A job waiting for the worker would keep a closed document alive. + #[cfg(not(target_arch = "wasm32"))] + { + app.preflight.worker.waiting = None; + for done in app.preflight.worker.poll() { + app.preflight.keep(done); + } + } + return None; + }; let cache = Arc::clone(&app.session.cache); let state = &mut app.preflight; #[cfg(not(target_arch = "wasm32"))] @@ -355,13 +369,17 @@ fn spawn(ctx: egui::Context, live: &Arc) -> Opti .name("designcraft-preflight".into()) .spawn(move || { let _alive = alive; - while let Ok(job) = jobs.recv() { - let started = Instant::now(); - let issues = check_document(&job.doc, &job.cache, job.of.path.as_deref().map(std::path::Path::new), MIN_PPI); - let found = Found { of: job.of, issues: Arc::new(issues), took: started.elapsed(), asked_at: 0.0 }; - // The checked state isn't kept alive by its check once the result is sent. - drop(job.doc); - if done_tx.send(found).is_err() { + let mut own = None; + loop { + let job = match jobs.recv_timeout(FORGET_AFTER) { + Ok(job) => job, + Err(std::sync::mpsc::RecvTimeoutError::Timeout) => { + own = None; + continue; + } + Err(std::sync::mpsc::RecvTimeoutError::Disconnected) => break, + }; + if done_tx.send(check(&mut own, job)).is_err() { break; } ctx.request_repaint(); @@ -371,6 +389,28 @@ fn spawn(ctx: egui::Context, live: &Arc) -> Opti Some((tx, rx)) } +/// Checks `job` with the worker's own compositions, `own`. +#[cfg(not(target_arch = "wasm32"))] +fn check(own: &mut Option<(u64, Cache)>, job: Job) -> Found { + let started = Instant::now(); + let issues = check_document(&job.doc, reader(own, &job), job.of.path.as_deref().map(std::path::Path::new), MIN_PPI); + // The checked state isn't kept alive by its check once the result is sent. + drop(job.doc); + Found { of: job.of, issues: Arc::new(issues), took: started.elapsed(), asked_at: 0.0 } +} + +/// The cache a worker checks `job` with: a cache over the interface thread's cache, which the +/// worker reads and never writes. The worker keeps it for the next job of the same document over +/// the same cache. +#[cfg(not(target_arch = "wasm32"))] +fn reader<'a>(own: &'a mut Option<(u64, Cache)>, job: &Job) -> &'a Cache { + let reuse = own.as_ref().is_some_and(|(uid, c)| *uid == job.of.uid && c.base().is_some_and(|b| Arc::ptr_eq(b, &job.cache))); + if !reuse { + *own = None; + } + &own.get_or_insert_with(|| (job.of.uid, Cache::over(Arc::clone(&job.cache)))).1 +} + /// Starts the worker in a test, where checks otherwise run on the interface thread. #[cfg(all(test, not(target_arch = "wasm32")))] pub(crate) fn start_worker(app: &mut DesignApp, ctx: &egui::Context) { diff --git a/crates/ui-egui/src/preflight_worker/tests.rs b/crates/ui-egui/src/preflight_worker/tests.rs index f437a4174..9cc9a394b 100644 --- a/crates/ui-egui/src/preflight_worker/tests.rs +++ b/crates/ui-egui/src/preflight_worker/tests.rs @@ -212,6 +212,53 @@ fn a_blocked_check_of_another_document_is_superseded() { assert!(jobs.try_recv().is_ok(), "queued on the same worker"); } +/// A worker checking an older state of the document leaves the interface thread's composition of +/// the current state in its cache. +#[cfg(not(target_arch = "wasm32"))] +#[test] +fn a_check_of_an_older_state_keeps_the_current_compositions() { + let (mut app, id) = overset_app(); + let (older, _) = Checked::new(&app).unwrap(); + let older_doc = Arc::clone(&app.session.active().unwrap().doc); + app.run("transform.move", json!({"ids": [id], "dx": 1.0, "dy": 0.0})).unwrap(); + let doc = Arc::clone(&app.session.active().unwrap().doc); + let sid = *doc.stories.keys().next().unwrap(); + let cache = Arc::clone(&app.session.cache); + let current = cache.get(&doc, sid, None); + let mut own = None; + let found = super::check(&mut own, super::Job { of: older, doc: Arc::clone(&older_doc), cache: Arc::clone(&cache) }); + assert_eq!(found.issues.iter().filter(|i| i.severity == "error").count(), 1); + assert!(Arc::ptr_eq(&cache.get(&doc, sid, None), ¤t), "the current state's composition is kept"); + // The worker keeps its compositions for the next check of the document over the same cache. + let earlier = own.as_ref().unwrap().1.get(&older_doc, sid, None); + let (of, doc) = Checked::new(&app).unwrap(); + super::check(&mut own, super::Job { of, doc, cache: Arc::clone(&cache) }); + assert!(Arc::ptr_eq(&own.as_ref().unwrap().1.get(&older_doc, sid, None), &earlier), "kept for the next check"); + let reloaded = Arc::new(designcraft_compose::Cache::new()); + let (of, doc) = Checked::new(&app).unwrap(); + super::check(&mut own, super::Job { of, doc, cache: Arc::clone(&reloaded) }); + assert!(own.as_ref().unwrap().1.base().is_some_and(|b| Arc::ptr_eq(b, &reloaded)), "a new cache after a font reload"); +} + +/// Closing the last document drops the job waiting for a busy worker, which holds that document. +#[cfg(not(target_arch = "wasm32"))] +#[test] +fn closing_the_last_document_drops_the_waiting_job() { + let (mut app, id) = overset_app(); + let ctx = egui::Context::default(); + let (_jobs, _done) = stalled_worker(&mut app); + for dx in [1.0, -1.0] { + app.run("transform.move", json!({"ids": [id], "dx": dx, "dy": 0.0})).unwrap(); + assert!(super::issues(&mut app, &ctx).is_none()); + } + assert!(app.preflight.worker.waiting.is_some()); + app.run("file.close", json!({})).unwrap(); + assert!(app.session.active().is_none()); + assert!(super::issues(&mut app, &ctx).is_none()); + assert!(app.preflight.worker.waiting.is_none(), "the waiting job is dropped"); + assert!(app.preflight.found.is_empty()); +} + /// A result for another document keeps the active document's issues, and the issues of at most /// `MAX_DOCUMENTS` documents are kept. #[cfg(not(target_arch = "wasm32"))]