mirror of
https://github.com/Myriad-Dreamin/tinymist.git
synced 2025-11-24 05:06:41 +00:00
fix(tinymist-project): invalidate cached snapshot after compile (#2057)
Some checks are pending
tinymist::auto_tag / auto-tag (push) Waiting to run
tinymist::ci / Duplicate Actions Detection (push) Waiting to run
tinymist::ci / Check Clippy, Formatting, Completion, Documentation, and Tests (Linux) (push) Waiting to run
tinymist::ci / Check Minimum Rust version and Tests (Windows) (push) Waiting to run
tinymist::ci / prepare-build (push) Waiting to run
tinymist::ci / announce (push) Blocked by required conditions
tinymist::ci / build (push) Blocked by required conditions
tinymist::gh_pages / build-gh-pages (push) Waiting to run
Some checks are pending
tinymist::auto_tag / auto-tag (push) Waiting to run
tinymist::ci / Duplicate Actions Detection (push) Waiting to run
tinymist::ci / Check Clippy, Formatting, Completion, Documentation, and Tests (Linux) (push) Waiting to run
tinymist::ci / Check Minimum Rust version and Tests (Windows) (push) Waiting to run
tinymist::ci / prepare-build (push) Waiting to run
tinymist::ci / announce (push) Blocked by required conditions
tinymist::ci / build (push) Blocked by required conditions
tinymist::gh_pages / build-gh-pages (push) Waiting to run
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 <camiyoru@gmail.com>
This commit is contained in:
parent
6573ed917b
commit
66351570a6
1 changed files with 34 additions and 27 deletions
|
|
@ -409,13 +409,11 @@ impl<F: CompilerFeat + Send + Sync + 'static, Ext: Default + 'static> 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<F: CompilerFeat + Send + Sync + 'static, Ext: Default + 'static> 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<F: CompilerFeat, Ext> {
|
|||
pub export_target: ExportTarget,
|
||||
/// The reason to compile.
|
||||
pub reason: CompileSignal,
|
||||
/// The latest compute graph (snapshot).
|
||||
snapshot: Option<Arc<WorldComputeGraph<F>>>,
|
||||
/// The latest compilation.
|
||||
pub compilation: OnceLock<CompiledArtifact<F>>,
|
||||
/// The compilation handle.
|
||||
pub handler: Arc<dyn CompileHandler<F, Ext>>,
|
||||
/// The file dependencies.
|
||||
deps: EcoVec<ImmutPath>,
|
||||
|
||||
/// The latest successly compiled document.
|
||||
latest_success_doc: Option<TypstDocument>,
|
||||
latest_compilation: Option<CompilationState>,
|
||||
/// The latest compute graph (snapshot), derived lazily from
|
||||
/// `latest_compilation` as needed.
|
||||
cached_snapshot: Option<Arc<WorldComputeGraph<F>>>,
|
||||
}
|
||||
|
||||
committed_revision: usize,
|
||||
/// Information about a completed compilation.
|
||||
struct CompilationState {
|
||||
revision: usize,
|
||||
/// The document, if it compiled successfully.
|
||||
doc: Option<TypstDocument>,
|
||||
}
|
||||
|
||||
impl<F: CompilerFeat, Ext: 'static> ProjectInsState<F, Ext> {
|
||||
/// Creates a snapshot of the project.
|
||||
/// Gets a snapshot of the project.
|
||||
pub fn snapshot(&mut self) -> Arc<WorldComputeGraph<F>> {
|
||||
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<WorldComputeGraph<F>> {
|
||||
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<F: CompilerFeat, Ext: 'static> ProjectInsState<F, Ext> {
|
|||
})
|
||||
}
|
||||
|
||||
/// 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<F: CompilerFeat, Ext: 'static> ProjectInsState<F, Ext> {
|
|||
fn process_compile(&mut self, artifact: CompiledArtifact<F>) -> 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()) {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue