Preflight: the worker keeps its compositions out of the interface thread's cache

The worker checks the document state it was given, which is often one
edit behind the interface. It composed that older state into the shared
composition cache, replacing the interface thread's composition of the
current state, so the next frame composed the edited story again (about
14 ms per edit for a 10-page story).

Cache::over makes a cache that uses another cache's compositions of the
same state and keeps whatever it composes to itself. The worker checks
with one over the interface thread's cache, keeps it for the next check
of the same document, and drops it after a minute without jobs.

With no active document, the job waiting for a busy worker is dropped,
so it no longer keeps a closed document in memory until another
document opens.

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<HashMap<StoryId, (Vec<usize>, Arc<ComposedStory>)>>,
+    /// 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<Arc<Cache>>,
 }
 
 /// 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<Cache>) -> Self {
+        Cache { base: Some(base), ..Self::default() }
+    }
+
+    /// The cache given to [`Cache::over`].
+    pub fn base(&self) -> Option<&Arc<Cache>> {
+        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

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), &current), "the base's composition is used");
+    let earlier = reader.get(&old, sid, None);
+    assert!(!Arc::ptr_eq(&earlier, &current));
+    assert!(Arc::ptr_eq(&reader.get(&old, sid, None), &earlier), "kept in the reader");
+    assert!(Arc::ptr_eq(&base.get(&new, sid, None), &current), "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]) {

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<Arc<Vec
 }
 
 fn issues_at(app: &mut DesignApp, ctx: &egui::Context, now: f64) -> Option<Arc<Vec<Issue>>> {
-    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<std::sync::atomic::AtomicUsize>) -> 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<std::sync::atomic::AtomicUsize>) -> 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) {

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), &current), "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"))]