Initial persistent caching prototype by ibraheemdev · Pull Request #967 · salsa-rs/salsa

ibraheemdev · 2025-08-08T03:42:01Z

The overall design is laid out in this document. All the new functionality is gated under the persistence feature. However, this PR has a couple of limitations:

All transitive dependencies are forced to be serializable.
There is no easy way to update inputs after they are deserialized.
Query hashing is not implemented.
Accumulators and supertypes are not serializable.

netlify · 2025-08-08T03:42:06Z

✅ Deploy Preview for salsa-rs canceled.

Name	Link
🔨 Latest commit	`4103798`
🔍 Latest deploy log	https://app.netlify.com/projects/salsa-rs/deploys/6896b522cda9d500081f1913

ibraheemdev · 2025-08-08T03:45:08Z

+                deserialize_fn: Some(deserialize_fn),
+            }) => Some(quote! { (#serialize_fn, #deserialize_fn) }),
+            // Fallback to the `{Serialize, Deserialize}` implementations tuples if only one of the
+            // two is provided.


There is a little bit of weirdness here in that all of the following are accepted:

#[salsa::tracked(serialize)] #[salsa::tracked(deserialize)] #[salsa::tracked(serialize, deserialize)] #[salsa::tracked(serialize = ...)] #[salsa::tracked(deserialize = ...)] #[salsa::tracked(serialize = ..., deserialize = ...)] struct ...;

But I'm not sure disallowing any of those is better. With the derive-macro API, we should be able to get rid of this anyways and just use serde::{Serialize, Deserialize} directly.

I moved this into a persist attribute to make things more consistent:

#[salsa::tracked(persist)] #[salsa::tracked(persist(serialize = ...))] #[salsa::tracked(persist(deserialize = ...))] #[salsa::tracked(persist(serialize = ..., deserialize = ...))] struct ...;

We don't accept the serialize and deserialize arguments on tracked functions because both the input and output types need to implement serde::{Serialize, Deserialize}, and the arguments would be ambiguous.

codspeed-hq · 2025-08-08T03:46:53Z

CodSpeed Performance Report

Merging #967 will improve performances by 8.88%

_{Comparing ibraheemdev:ibraheem/persistent-caching (4103798) with master (940f9c0)}

Summary

⚡ 3 improvements
✅ 9 untouched benchmarks

Benchmarks breakdown

	Benchmark	`BASE`	`HEAD`	Change
⚡	`amortized[Input]`	2.3 µs	2.1 µs	+7.39%
⚡	`amortized[InternedInput]`	2.3 µs	2.1 µs	+8.88%
⚡	`new[InternedInput]`	4.2 µs	4 µs	+5.43%

ibraheemdev · 2025-08-08T04:14:39Z

+            let DeserializeIngredients(zalsa) = self;
+
+            while let Some(name) = access.next_key::<&str>()? {
+                // TODO: Serialize the ingredient index directly.


Technically the debug name is not a unique identifier, but I found it more helpful for debugging than the ingredient index. It could also just be a field on each ingredient.

A separate (maybe even optinal) field would make sense to me.

I removed the debug name for now, it's unfortunately very annoying to add extra fields because serde's macros aren't compatible with DeserializeSeed (which we need for some fields, because we deserialize into the table directly).

MichaReiser

This is great. I didn't read line by line and instead focused on the overall design.

What's your plan with this PR? Do you want to land it behind a persistence feature or do you want to keep iterating in a separate branch?

MichaReiser · 2025-08-08T12:10:18Z

+                // Ensure structs are serialized before tracked functions, as deserializing a
+                // memo requires its input struct to have been deserialized.


It's fine deferring this to later but that will mean that we always serialize all tracked structs even if some query instances are never serialized (that created those tracked structs)

MichaReiser · 2025-08-08T12:14:57Z

+            let DeserializeIngredients(zalsa) = self;
+
+            while let Some(name) = access.next_key::<&str>()? {
+                // TODO: Serialize the ingredient index directly.


A separate (maybe even optinal) field would make sense to me.

MichaReiser · 2025-08-08T12:32:40Z

+    <dyn salsa::Database>::deserialize(
+        &mut db,
+        &mut serde_json::Deserializer::from_str(&serialized),
+    )


Does deserialize bump the revision to 2 to force a validation of all queries?

Hmm no, should it?

I think there are a few things to consider here

memos and other ingredients that weren't verified in the last revision (current when deserializing) still need to be dirty. The easiest here might be to only serialize ingredients that are verified in the current revision and we revisit this later

we need to bump the revision if any input changes during deserialization. Now, we don't support this yet so it's fine to ignore for now.

memos and other ingredients that weren't verified in the last revision (current when deserializing) still need to be dirty. The easiest here might be to only serialize ingredients that are verified in the current revision and we revisit this later

We serialize and deserialize the current revision of the database, so I don't think this is a problem. Any ingredients that weren't verified in the last revision will have to be verified after deserialization.

ibraheemdev · 2025-08-08T21:22:22Z

I gated everything under a persistence feature. We should rework CI to use something like cargo-hack to test all the new feature permutations, but I'll do that in a separate PR.

ibraheemdev force-pushed the ibraheem/persistent-caching branch from 8931c07 to caf7925 Compare August 8, 2025 03:43

ibraheemdev commented Aug 8, 2025

View reviewed changes

ibraheemdev force-pushed the ibraheem/persistent-caching branch 5 times, most recently from 77b1f21 to 15a3110 Compare August 8, 2025 04:08

ibraheemdev commented Aug 8, 2025

View reviewed changes

MichaReiser approved these changes Aug 8, 2025

View reviewed changes

MichaReiser requested a review from nikomatsakis August 8, 2025 12:33

ibraheemdev force-pushed the ibraheem/persistent-caching branch from d30ee9d to 5e1a984 Compare August 8, 2025 21:06

ibraheemdev added 5 commits August 8, 2025 17:15

persistent caching prototype

d20d844

move serialization arguments under persist attribute

6b2c642

move serde dependency under persistence feature

f126c28

avoid serializing provisional memos

28c3960

use exhaustive checking for manual Serialize implementations

03377ef

ibraheemdev force-pushed the ibraheem/persistent-caching branch from 7c469e0 to 03377ef Compare August 8, 2025 21:15

ibraheemdev force-pushed the ibraheem/persistent-caching branch from 206d738 to 76a794b Compare August 8, 2025 22:35

ibraheemdev added 3 commits August 8, 2025 18:36

update tests

ccfd31c

remove distinction between ingredient entries and instances

bb6a676

avoid enabling shuttle feature in CI

b7e5658

ibraheemdev force-pushed the ibraheem/persistent-caching branch from 76a794b to b7e5658 Compare August 8, 2025 22:36

serialize ingredients by index

4103798

ibraheemdev force-pushed the ibraheem/persistent-caching branch from fecc37d to 4103798 Compare August 9, 2025 02:40

ibraheemdev added this pull request to the merge queue Aug 11, 2025

Merged via the queue into salsa-rs:master with commit 1ffb32f Aug 11, 2025
12 checks passed

ibraheemdev deleted the ibraheem/persistent-caching branch August 11, 2025 19:12

github-actions Bot mentioned this pull request Aug 8, 2025

chore: release v0.24.0 #929

Merged

ibraheemdev mentioned this pull request Aug 12, 2025

Flatten unserializable query dependencies #975

Merged

github-actions Bot mentioned this pull request Oct 9, 2025

chore: release v0.25.0 #1002

Merged

NeuralModder mentioned this pull request Jan 1, 2026

Serialization to disk #10

Open

		// Ensure structs are serialized before tracked functions, as deserializing a
		// memo requires its input struct to have been deserialized.

Conversation

ibraheemdev commented Aug 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

netlify Bot commented Aug 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for salsa-rs canceled.

Uh oh!

ibraheemdev Aug 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codspeed-hq Bot commented Aug 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging #967 will improve performances by 8.88%

Summary

Benchmarks breakdown

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MichaReiser left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ibraheemdev commented Aug 8, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ibraheemdev commented Aug 8, 2025 •

edited

Loading

netlify Bot commented Aug 8, 2025 •

edited

Loading

ibraheemdev Aug 8, 2025 •

edited

Loading

codspeed-hq Bot commented Aug 8, 2025 •

edited

Loading