Coarse-grained tracked structs

[nikomatsakis]

As described in this hackmd, we want to change how tracked struct dependencies work. The basic idea is to switch the default: instead of labeling some fields as #[id] and having the rest be independently tracked, we will have fields be #[id] by default unless they are annotated with #[salsa::tracked]. This is a fairly straightforward change that can be done in the proc-macro.

Example

Before:

#[salsa::tracked]
struct Function {
    #[id]
    name: String,
    body: String,
}

After:

#[salsa::tracked]
struct Function {
    name: String,

    #[salsa::tracked]
    body: String,
}

[MichaReiser]

CC: @ibraheemdev

[nikomatsakis]

Mentoring instructions

Ok, I realized this is a bit more complicated than I thought. There are two parts required

1. Swap the default.
2. Change how we track reads of #[id] fields -- we don't currently distinguish them, but we should.

swap the default

The decision about which fields are "id" fields is driven by this input to the macro-rules macro:

https://github.com/salsa-rs/salsa/blob/master/components/salsa-macro-rules/src/setup_tracked_struct.rs#L32-L33

which is provided here

salsa/components/salsa-macros/src/tracked_struct.rs

Line 112 in c6c51a0

id_field_indices: [#(#id_field_indices),*],

it should be fairly straightforward to modify the latter file to flip the defaults. (Note that the next section will also likely change this interface slightly.)

I'd probably also rename these from "id" fields to "untracked" fields.

change how we track reads of #[id] fields

This is the trickier bit. Currently we create a field ingredient for every field, regardless of whether it's part of the #[id] hash:

salsa/src/tracked_struct.rs

Lines 101 to 115 in c6c51a0

	impl<C: Configuration> Jar for JarImpl<C> {
	fn create_ingredients(
	&self,
	_aux: &dyn JarAux,
	struct_index: crate::zalsa::IngredientIndex,
	) -> Vec<Box<dyn Ingredient>> {
	let struct_ingredient = <IngredientImpl<C>>::new(struct_index);
	std::iter::once(Box::new(struct_ingredient) as _)
	.chain((0..C::FIELD_DEBUG_NAMES.len()).map(|field_index| {
	Box::new(<FieldIngredientImpl<C>>::new(struct_index, field_index)) as _
	}))
	.collect()
	}
	}

the field accessor for every field is the same:

salsa/components/salsa-macro-rules/src/setup_tracked_struct.rs

Lines 198 to 212 in c6c51a0

	$(
	$field_getter_vis fn $field_getter_id<$Db>(self, db: &$db_lt $Db) -> $crate::maybe_cloned_ty!($field_option, $db_lt, $field_ty)
	where
	// FIXME(rust-lang/rust#65991): The `db` argument *should* have the type `dyn Database`
	$Db: ?Sized + $zalsa::Database,
	{
	let db = db.as_dyn_database();
	let fields = $Configuration::ingredient(db).field(db, self, $field_index);
	$crate::maybe_clone!(
	$field_option,
	$field_ty,
	&fields.$field_index,
	)
	}
	)*

in particular it invokes the field method from the tracked struct on line 205. That function is defined here:

salsa/src/tracked_struct.rs

Lines 516 to 524 in c6c51a0

	/// Access to this value field.
	/// Note that this function returns the entire tuple of value fields.
	/// The caller is responible for selecting the appropriate element.
	pub fn field<'db>(
	&'db self,
	db: &'db dyn crate::Database,
	s: C::Struct<'db>,
	field_index: usize,
	) -> &'db C::Fields<'db> {

It does a fair bit of work and in particular adds a read.

All of that work is unnecessary for id-fields. Just as with an interned struct, the idea is that if the id fields have changed, then you will have a new tracked struct instance from salsa's perspective, so they can be regarded as immutable. As a result, we just have to read from them. Compare to the fields method from interned structs.

I think what I would do is to modify the setup_tracked_struct macro so that when we generate the field getters, we generate the getters for id fields differently (they invoke a untracked_fields method on the ingredient or something like that, and we rename the existing getter to tracked_fields).

This will require changing the macro inputs. There are a few few ways to go about this. Perhaps the easiest is to add some kind of field_getter_method argument that is either untracked_fields or tracked_fields depending and then modify this line

salsa/components/salsa-macro-rules/src/setup_tracked_struct.rs

Line 205 in c6c51a0

let fields = $Configuration::ingredient(db).field(db, self, $field_index);

to read let fields = $Configuration::ingredient(db).$field_getter_method(db, self, $field_index);.

[ibraheemdev]

Thanks for the writeup! I can work on this after #597 (if no one else has gotten to it first).

[OLUWAMUYIWA]

Picking this up if you haven't @ibraheemdev

[ibraheemdev]

@OLUWAMUYIWA Feel free!