RFD: predicate caching#262
Conversation
Add design for caching predicate results using file-based watch paths. Two mechanisms: syntactic (watch field on skill groups) and runtime (stdout JSON from shell predicates). Coordinates with PM interface for depends-on() caching via Cargo.lock.
nikomatsakis
left a comment
There was a problem hiding this comment.
Thanks! Left some notes here.
|
|
||
| Two ways to declare watch paths: | ||
|
|
||
| ### 1. Syntactic |
There was a problem hiding this comment.
Hmm. I'm inclined to skip this and just focus on the runtime version. I guess that would make something like a shell predicate kind of inherently tricky but that's ok, I don't think an arbitrary shell command is a very good idea for portability reasons anyhow. I'd rather we have declarative forms for common shell commands.
There was a problem hiding this comment.
I'll drop the syntactic watch field and focus on the JSONL runtime approach. Agreed that arbitrary shell commands aren't great for portability and maybe declarative forms for common patterns is the better direction.
| The shell command emits JSON to stdout: | ||
|
|
||
| ```json | ||
| {"result": true, "watch": ["CargoBrazil.toml"]} |
There was a problem hiding this comment.
I think we restructured the custom predicates to emit lines of jsonl, right? I think that the idea would be that they emit a line like
{"watch": {
"files": ["CargoBrazil.toml"]
}}
which would derive from
#[derive(SErialize, Deserialize)]
enum CustomPredicateEvent {
...., // whatever other events we have, I think right now that is for selected crates? I think we'll be removing that eventually but it's ok
Watch { files: Vec<PathBuf> }
}I believe that the predicate result comes from the exit status of the shell? Anyway, it should be independent from the custom predicate event.
There was a problem hiding this comment.
I'll align with the existing CustomPredicateEvent enum and make
Watch { files: Vec<PathBuf> }
a variant. Exit status stays as the result, watch event is just a cache hint emitted alongside. Will update the RFD.
|
|
||
| Fingerprint is `mtime + size` (same model as Cargo). A missing file is a valid fingerprint state — invalidates when the file appears. | ||
|
|
||
| Cache lives at `predicate-cache.json`, keyed by the normalized predicate string. Two plugins with the same predicate share one entry. |
There was a problem hiding this comment.
I think the key part is that the cache is stored in the user's symposium directory? I would assume we are kind of generalizing the current Cargo.lock caching mechanism.
There was a problem hiding this comment.
Got it, I'll put it in ~/.symposium/cache/. Since watch paths are resolved relative to the workspace root at stat time, the fingerprint comparison naturally should handle multi-workspace.
| - `workspace-member()` → cached, watch = [] (static per run) | ||
| - `path_exists(path)` → cached, watches the path itself | ||
| - `depends-on(name)` → cached, watches PM manifest (e.g. `Cargo.lock`) | ||
| - `env(...)` → never cached (env isn't file-observable) |
There was a problem hiding this comment.
I think we should add another kind of predicate here - not just files but also the values of environment variables. Env can cache this way and shell predicates should also be able to emit that. We could have a helper in the symposium-sdk crate for reading from the environment that automatically emits the cache line too, so that when you read an env variable you are always cached on its value.
d1f0781 to
d699619
Compare
nikomatsakis
left a comment
There was a problem hiding this comment.
Much better! Left a few more comments.
| ``` | ||
|
|
||
| `watch` is a sibling field, not embedded in the predicate string. | ||
| The predicate result still comes from exit status — the watch event is just a cache hint. Predicates that don't emit a `Watch` event are never cached (current behavior preserved). |
There was a problem hiding this comment.
I think it should be the opposite. If you don't emit watch hint(s), you are ALWAYS cached, because we assume we don't have to cache.
Also, let's make it clear that predicates can emit multiple watch hints that are all unioned together.
| struct DepsResult { | ||
| ids: Vec<PackageId>, | ||
| watch: Option<Vec<WatchEntry>>, | ||
| watch: Option<Vec<PathBuf>>, |
There was a problem hiding this comment.
I don't think these should be optional. Also, do we need environments here?
I'm not exactly clear on the role of DepsResult, I guess I have to go digging.
What does this PR do?
RFD for predicate caching. Shell predicates and custom predicates currently fork a process on every sync and this adds a mechanism for predicates to declare file-based watch paths so Symposium can cache results and skip re-evaluation when watched files haven't changed.
Two mechanisms:
Questions for reviewers
Disclosure questions
AI disclosure.I used an AI tool for research, autocomplete, or in other minimal ways.
AI tool authored most of this RFD