feat: add previous() to access a tracked function's prior memoized value

[Veykril]

Introduce a generated previous() associated function on every tracked
function, returning a reference to the value memoized for the active key
in the previous revision (or None if the function has never completed
for that key, or its prior result is provisional).

This lets a tracked function build its new result on top of its own
prior result while re-executing. Reading the previous value replays that
result's dependencies into the current execution — durability, dependency
edges, untracked-read flag, changed-at revision, and tracked-struct ids —
so incremental validation stays correct and previously-created tracked
structs remain live.

[netlify]

✅ Deploy Preview for salsa-rs canceled.

Name	Link
🔨 Latest commit	d089052
🔍 Latest deploy log	https://app.netlify.com/projects/salsa-rs/deploys/6a479bf470dd0800082a5b8b

[Veykril]

@MichaReiser this is the feature i was hinting at (I just revived the branch on top of main). There are still quite a bit of unknowns that I have to think about in terms of whether this is sound and what exactly it means to read your previous memo in a query, but opus thinks its safe (and iirc codex which I used to write this back then agreed).

[MichaReiser]

Yeah, I think this should probably be fine (you already handle fixpoint queries, which was my main concern :))

So this could e.g. be used for an incremental parse?

[Veykril]

Also I haven't given the tests a proper look yet (they are llm generated) and there are likely scenarios that are missing still.

As for why r-a is interested in this, see rust-lang/rust-analyzer#22532. Basically on trivia edits, we don't want to re-execute the proc-macros, instead we'd like to just take the previous result, fix up the spans manually and emit that as the new memo.

So yea, this is useful for more manual incremental recomputation handling. Though on second thought, this alone won't be enough since we need to know the diff of the inputs for that 🤔 Nevermind I think I remember, we'd store that info in the input when building it in its given query. Bit iffy on the details, would need to test drive that to know

[codspeed-hq]

Merging this PR will degrade performance by 87.24%

⚡ 2 improved benchmarks
❌ 4 regressed benchmarks
✅ 45 untouched benchmarks
⏩ 12 skipped benchmarks¹

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

	Mode	Benchmark	BASE	HEAD	Efficiency
❌	Simulation	input_field_read_many	259.8 µs	2,355,772.4 µs	-99.99%
❌	Simulation	interned_field_read_many	210.7 µs	6,233.3 µs	-96.62%
❌	Simulation	tracked_field_read_many	270.8 µs	514.7 µs	-47.38%
❌	Simulation	untracked_field_read_many	222.4 µs	374.4 µs	-40.61%
⚡	Simulation	deep_verify_then_execute	4.2 ms	2.2 ms	+96.13%
⚡	Simulation	cold_execute	3.1 ms	1.7 ms	+88.61%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.

---

_{Comparing Veykril:lukaswirth/push-znzskroxnsym (d089052) with master (049c3c4)}

Footnotes

1. 12 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

[Veykril]

Calling out the specify case, we could recognize this and have the function return something else here (enum distinguishing the cases) but I don't think thats worth it until useful to someone.

[MichaReiser]

I agree with your reasoning here. The idea of specify is that it's mostly transparent. So handling them as described here makes sense to me

[Veykril]

I think I've covered everything now (forgot about accumulators which is now fixed)

[MichaReiser]

I haven't reviewed the code yet. But I wonder if previous should also return the current memo value for fixpoint iteration. It seems equally useful to avoid rebuilding the entire struct (for as long as it's only read-only). previous would always be the previous memo: Either from the last iteration or last revision

[Veykril]

Can we just return it or would that require more plumbing? Not familiar with how fixpoint deals with its memos

[MichaReiser]

Let me have a look at the code. I'm not sure what you mean by just returning it :)

Overall, the memos are handled mostly the same as regular memos. Except that we replace them after each iteration

[MichaReiser]

Probably not what you intended it for, but I have a use case for this in ty to basically check: Has this been computed, if so, call the query, otherwise do something else.

I have to think this through a bit more carefully, but I don't think this is sound for non 'static values. That is, any value that contains anything with a 'db lifetime, because you can now read values as they're been updated or that haven't been verified yet (or GCed).

Another issue is that calling previous multiple times calls seed_tracked_struct multiples times.

[MichaReiser]

Do we need to attach here? Is it mainly to assert the same-db constraint?

[MichaReiser]

I need to land my monomorphization PR, so that we can move this out of the C block :)

[MichaReiser]

Hmm, I'm not sure this is sound. I have to think this through a bit more carefully.

There's no guarantee that the input edges from a previous revision are still valid. That is, some input edges may no longer exist because they were garbage collected, or were never deserialized after persistent caching.

Carrying these dependencies forward could also mean that a query now depends on the same IDs that only differ b y generations.

[MichaReiser]

Maybe it's best to exclude provisional memos for now, even if they're from the previous revision (it's not guaranteed that all memos participating in a cycle are finalized).

For same-revisional provisionals, we'd have to carry over the cycle heads. Ultimately, this is the same as calling self again (fixpoint!). It's a bit less clear to me what needs to happen when depending on a provisional from a previous revision.

[MichaReiser]

What's unclear to me is whether this is safe if value contains tracked structs that were created by the active query. You can now read tracked structs that are being updated as your query runs. In general, you can now read values that are potentially outdated, because they haven't been verified yet (or contain interned values that were garbage collected)

[MichaReiser]

I think restricting values to 'static should solve the tracked struct and interned struct issue. I'm not sure how to solve the dependency issue.

It's not even clear to me what the expected dependencies should be. Depending on all previous dependencies is an over-approximation. I also doesn't protect against missing new dependencies. E.g. a query could clone parts of the result and skip a struct read, which it normally would depend on if previous isn't provided. IT also has the issue that it's possible that the query is immediately out dated again because one of its dependency hasn't been verified.

I sort of think that we may need to iterate over all dependencies and repeat some of deep_verify_memo, but in a slightly different way?

• For interned: If an interned changed, it means it was garbage collected. We need to bail, because later edges could be queries that were stored on this interned struct (which no longer exists). We need to re-execute the query to recreate all interned structs.
• Inputs: Fine, carry over?
• Query: Do the same as deep_verify_memo. That is, rerun the query. This ensures that maybe_changed_after will return false right after the outer query completed.

The part that remains unclear to me are tracked structs. But I'd be fine to limit this feature to return None if the query created any tracked structs. The issue with tracked structs is the same as with interned. We don't know if the tracked struct will be collected after the query completes. But collecting them would tear down queries that previous dependent on...

To some extend, this reminds me of fixpoint where previous is a very educated guess of what the initial value is. Now, we can't use fixpoint because the results aren't guaranteed to be monotonic and this is also a stricter form where queries are only allowed to depend on their own previous result, other queries aren't allowed to form a cycle.