From 66351570a6f6aee097fefdcfb776316a27adefb6 Mon Sep 17 00:00:00 2001 From: Joseph Liu Date: Tue, 26 Aug 2025 12:45:38 -0400 Subject: [PATCH] fix(tinymist-project): invalidate cached snapshot after compile (#2057) This PR fixes a latent cache invalidation issue with project snapshots. I recommend reviewing commit-by-commit, even though this is a small change. Only the last commit changes behavior; the rest are cleaning up code to make it easier to see what is happening. --- Here's what I think is the problem. Project instance states (`ProjectInsState`) provide access to a snapshot of the latest compilation result. This snapshot is cached for performance: ```rust match self.snapshot.as_ref() { Some(snap) if snap.world().revision() == self.verse.revision => snap.clone(), // otherwise recompute } ``` Unfortunately, it turns out that the cached snapshot may be outdated even if the revision matches. (In practice, it will usually just lag one compilation behind, and I think this is why it hasn't caused any observable issues--one compilation behind is not that much, especially if the user is continuously typing.) To see the issue, consider the following sequence of steps. Assume initially all revisions are `0`. 1. User triggers a recompilation, say by typing into the file. - This triggers an `Interrupt::Memory` or similar. 2. The changes are applied to the vfs and recompilation begins. - `verse.revision` is incremented to `verse.revision = 1`. 3. Before compilation finishes, a project snapshot is requested. - The project compiler checks the cached snapshot first, which still has `snap.world().revision() == 0`. - Since `verse.revision = 1` was already incremented before the compilation finished, a new snapshot is created and cached--even though nothing has actually been recompiled yet. - The cached snapshot now has `snap.world().revision() == 1`. 4. Compilation finishes, and some data from the resulting compilation artifact is recorded. 5. Another project snapshot is requested. - Since the snapshot was recomputed already in (3), the revisions appear to match and the outdated, cached snapshot is returned. The fix here is to just clear the cached snapshot in (4) after recompilation finishes. --- I would add a regression test, but it didn't seem easy to write an automated test that can differentiate the change here. I tested this locally by adding some logging around when the cached snapshot is used and when it is recomputed. As mentioned on Discord, I noticed this problem when adding some more data to project snapshots on a local fork of Tinymist, which caused the slightly outdated state to become a little more obvious. --------- Co-authored-by: Myriad-Dreamin --- crates/tinymist-project/src/compiler.rs | 61 ++++++++++++++----------- 1 file changed, 34 insertions(+), 27 deletions(-) diff --git a/crates/tinymist-project/src/compiler.rs b/crates/tinymist-project/src/compiler.rs index 3f60eb7a..16d85b46 100644 --- a/crates/tinymist-project/src/compiler.rs +++ b/crates/tinymist-project/src/compiler.rs @@ -409,13 +409,11 @@ impl ProjectCom ext: Default::default(), verse, reason: no_reason(), - snapshot: None, handler, export_target, - compilation: OnceLock::default(), - latest_success_doc: None, deps: Default::default(), - committed_revision: 0, + latest_compilation: None, + cached_snapshot: None, } } @@ -548,8 +546,8 @@ impl ProjectCom }); } - // Reset the watch state and document state. - proj.latest_success_doc = None; + // Forget the old compilation state. + proj.latest_compilation = None; } proj.reason.merge(reason_by_entry_change()); @@ -748,46 +746,51 @@ pub struct ProjectInsState { pub export_target: ExportTarget, /// The reason to compile. pub reason: CompileSignal, - /// The latest compute graph (snapshot). - snapshot: Option>>, - /// The latest compilation. - pub compilation: OnceLock>, /// The compilation handle. pub handler: Arc>, /// The file dependencies. deps: EcoVec, - /// The latest successly compiled document. - latest_success_doc: Option, + latest_compilation: Option, + /// The latest compute graph (snapshot), derived lazily from + /// `latest_compilation` as needed. + cached_snapshot: Option>>, +} - committed_revision: usize, +/// Information about a completed compilation. +struct CompilationState { + revision: usize, + /// The document, if it compiled successfully. + doc: Option, } impl ProjectInsState { - /// Creates a snapshot of the project. + /// Gets a snapshot of the project. pub fn snapshot(&mut self) -> Arc> { - match self.snapshot.as_ref() { - Some(snap) if snap.world().revision() == self.verse.revision => snap.clone(), + // Tries to use the cached snapshot if possible. + match self.cached_snapshot.as_ref() { + Some(cached) if cached.world().revision() == self.verse.revision => cached.clone(), _ => { let snap = self.make_snapshot(); - self.snapshot = Some(snap.clone()); + self.cached_snapshot = Some(snap.clone()); snap } } } + /// Creates a new snapshot of the project derived from `latest_compilation`. fn make_snapshot(&self) -> Arc> { let world = self.verse.snapshot(); let snap = CompileSnapshot { id: self.id.clone(), world, signal: self.reason, - success_doc: self.latest_success_doc.clone(), + success_doc: self.latest_compilation.as_ref().and_then(|c| c.doc.clone()), }; WorldComputeGraph::new(snap) } - /// Compile the document once if there is any reason and the entry is + /// Compiles the document once if there is any reason and the entry is /// active. (this is used for experimenting typst.node compilations) #[must_use] pub fn may_compile2<'a>( @@ -806,7 +809,7 @@ impl ProjectInsState { }) } - /// Compile the document once if there is any reason and the entry is + /// Compiles the document once if there is any reason and the entry is /// active. #[must_use] pub fn may_compile( @@ -881,18 +884,22 @@ impl ProjectInsState { fn process_compile(&mut self, artifact: CompiledArtifact) -> bool { let world = &artifact.snap.world; let compiled_revision = world.revision().get(); - if self.committed_revision >= compiled_revision { + if let Some(cur) = &self.latest_compilation + && cur.revision >= compiled_revision + { return false; } - // Update state. + // Updates state. let doc = artifact.doc.clone(); - self.committed_revision = compiled_revision; - if doc.is_some() { - self.latest_success_doc = doc; - } + self.latest_compilation = Some(CompilationState { + revision: compiled_revision, + doc, + }); + // Invalidates the snapshot. It will be recomputed on demand. + self.cached_snapshot = None; - // Notify the new file dependencies. + // Notifies the new file dependencies. let mut deps = eco_vec![]; world.iter_dependencies(&mut |dep| { if let Ok(x) = world.file_path(dep).and_then(|e| e.to_err()) {