add comments, remove dead-code lints

components/salsa-2022/src/function.rs

@@ -26,11 +26,30 @@ mod set;
 mod store;
 mod sync;
 
+/// Function ingredients are the "workhorse" of salsa.
+/// They are used for tracked functions, for the "value" fields of tracked structs, and for the fields of input structs.
+/// The function ingredient is fairly complex and so its code is spread across multiple modules, typically one per method.
+/// The main entry points are:
+///
+/// * the `fetch` method, which is invoked when the function is called by the user's code;
+///   it will return a memoized value if one exists, or execute the function otherwise.
+/// * the `set` method, which can only be used when the key is an entity created by the active query.
+///   It sets the value of the function imperatively, so that when later fetches occur, they'll return this value.
+/// * the `store` method, which can only be invoked with an `&mut` reference, and is to set input fields.
 #[allow(dead_code)]
 pub struct FunctionIngredient<C: Configuration> {
+    /// The ingredient index we were assigned in the database.
+    /// Used to construct `DatabaseKeyIndex` values.
     index: IngredientIndex,
+
+    /// Tracks the keys for which we have memoized values.
     memo_map: memo::MemoMap<C::Key, C::Value>,
+
+    /// Tracks the keys that are currently being processed; used to coordinate between
+    /// worker threads.
     sync_map: sync::SyncMap,
+
+    /// Used to find memos to throw out when we have too many memoized values.
     lru: lru::Lru,
 
     /// When `fetch` and friends executes, they return a reference to the
@@ -50,17 +69,41 @@ pub struct FunctionIngredient<C: Configuration> {
 
 pub trait Configuration {
     type Jar: for<'db> Jar<'db>;
+
+    /// What key is used to index the memo. Typically a salsa struct id,
+    /// but if this memoized function has multiple argments it will be a `salsa::Id`
+    /// that results from interning those arguments.
     type Key: AsId;
+
+    /// The value computed by the function.
     type Value: std::fmt::Debug;
 
+    /// Determines whether this function can recover from being a participant in a cycle
+    /// (and, if so, how).
     const CYCLE_STRATEGY: CycleRecoveryStrategy;
 
+    /// Invokes after a new result `new_value`` has been computed for which an older memoized
+    /// value existed `old_value`. Returns true if the new value is equal to the older one
+    /// and hence should be "backdated" (i.e., marked as having last changed in an older revision,
+    /// even though it was recomputed).
+    ///
+    /// This invokes user's code in form of the `Eq` impl.
     fn should_backdate_value(old_value: &Self::Value, new_value: &Self::Value) -> bool;
 
+    /// Invoked when we need to compute the value for the given key, either because we've never
+    /// computed it before or because the old one relied on inputs that have changed.
+    ///
+    /// This invokes the function the user wrote.
     fn execute(db: &DynDb<Self>, key: Self::Key) -> Self::Value;
 
+    /// If the cycle strategy is `Recover`, then invoked when `key` is a participant
+    /// in a cycle to find out what value it should have.
+    ///
+    /// This invokes the recovery function given by the user.
     fn recover_from_cycle(db: &DynDb<Self>, cycle: &Cycle, key: Self::Key) -> Self::Value;
 
+    /// Given a salsa Id, returns the key. Convenience function to avoid
+    /// having to type `<C::Key as AsId>::from_id`.
     fn key_from_id(id: Id) -> Self::Key {
         AsId::from_id(id)
     }

components/salsa-2022/src/function/memo.rs

@@ -8,6 +8,9 @@ use crate::{
     EventKind, Revision, Runtime,
 };
 
+/// The memo map maps from a key of type `K` to the memoized value for that `K`.
+/// The memoized value is a `Memo<V>` which contains, in addition to the value `V`,
+/// dependency information.
 pub(super) struct MemoMap<K: AsId, V> {
     map: FxDashMap<K, ArcSwap<Memo<V>>>,
 }

components/salsa-2022/src/function/set.rs

@@ -1,7 +1,6 @@
 use crossbeam::atomic::AtomicCell;
 
 use crate::{
-    key::DependencyIndex,
     runtime::local_state::{QueryInputs, QueryRevisions},
     tracked_struct::TrackedStructInDb,
     Database,

components/salsa-2022/src/hash.rs

@@ -1,4 +1,3 @@
-#![allow(dead_code)]
 use std::hash::{BuildHasher, Hash, Hasher};
 
 pub(crate) type FxHasher = std::hash::BuildHasherDefault<rustc_hash::FxHasher>;

components/salsa-2022/src/interned.rs

@@ -20,7 +20,9 @@ impl<T: AsId> InternedId for T {}
 pub trait InternedData: Sized + Eq + Hash + Clone {}
 impl<T: Eq + Hash + Clone> InternedData for T {}
 
-#[allow(dead_code)]
+/// The interned ingredient has the job of hashing values of type `Data` to produce an `Id`.
+/// It used to store interned structs but also to store the id fields of a tracked struct.
+/// Interned values endure until they are explicitly removed in some way.
 pub struct InternedIngredient<Id: InternedId, Data: InternedData> {
     /// Index of this ingredient in the database (used to construct database-ids, etc).
     ingredient_index: IngredientIndex,
@@ -68,7 +70,6 @@ where
         }
     }
 
-    #[allow(dead_code)]
     pub fn intern(&self, runtime: &Runtime, data: Data) -> Id {
         runtime.report_tracked_read(
             DependencyIndex::for_table(self.ingredient_index),
@@ -101,7 +102,6 @@ where
         self.reset_at
     }
 
-    #[allow(dead_code)]
     pub fn reset(&mut self, revision: Revision) {
         assert!(revision > self.reset_at);
         self.reset_at = revision;
@@ -109,7 +109,6 @@ where
         self.value_map.clear();
     }
 
-    #[allow(dead_code)]
     #[track_caller]
     pub fn data<'db>(&'db self, runtime: &'db Runtime, id: Id) -> &'db Data {
         runtime.report_tracked_read(

components/salsa-2022/src/revision.rs

@@ -46,7 +46,6 @@ pub(crate) struct AtomicRevision {
     data: AtomicUsize,
 }
 
-#[allow(dead_code)]
 impl AtomicRevision {
     pub(crate) fn start() -> Self {
         Self {

components/salsa-2022/src/routes.rs

@@ -3,37 +3,66 @@ use super::{
     storage::HasJars,
 };
 
+/// An ingredient index identifies a particular [`Ingredient`] in the database.
+/// The database contains a number of jars, and each jar contains a number of ingredients.
+/// Each ingredient is given a unique index as the database is being created.
 #[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Debug)]
 pub struct IngredientIndex(u32);
 
 impl IngredientIndex {
+    /// Create an ingredient index from a usize.
     fn from(v: usize) -> Self {
         assert!(v < (std::u32::MAX as usize));
         Self(v as u32)
     }
 
+    /// Convert the ingredient index back into a usize.
     fn as_usize(self) -> usize {
         self.0 as usize
     }
 }
 
+/// A "route" is a function that, given a `&DB::Jars`, returns an `&dyn Ingredient`.
+/// Routes are constructed (in part) from closures generated by the salsa macros.
+/// These closures look essentially like `|jar| &jar.some_field` -- i.e., if a jar is a struct,
+/// the closure returns a reference to some particular field of the struct
+/// (whichever field has the database for this ingredient).
+///
+/// The key point here is: the struct definitions that are being referencd here come from
+/// crates that consume this crate, and hence we cannot name them directly.
+/// We have to navigate them through closures generated by that downstream crate.
 #[allow(type_alias_bounds)]
 #[allow(unused_parens)]
 pub type DynRoute<DB: HasJars> = dyn Fn(&DB::Jars) -> (&dyn Ingredient<DB>) + Send + Sync;
 
+/// Like a `DynRoute`, but for `&mut` references.
 #[allow(type_alias_bounds)]
 #[allow(unused_parens)]
 pub type DynMutRoute<DB: HasJars> =
     dyn Fn(&mut DB::Jars) -> (&mut dyn MutIngredient<DB>) + Send + Sync;
 
+/// The "ingredients" structure is used to navigate the database.
+/// The database contains a number of jars, and each jar contains a number of ingredients.
+/// When the database is created, it creates each jar in turn.
+/// Each jar then creates its ingredients.
+/// Each ingredient is registered with the database by invoking the [`Ingredients::push`] method.
+/// This method assigns it a unique [`IngredientIndex`] and stores some callbacks indicating
+/// how to find the ingredient later based only on the index.
 pub struct Ingredients<DB: HasJars> {
+    /// Vector indexed by ingredient index. Yields the `DynRoute`,
+    /// a function which can be applied to the `DB::Jars` to yield
+    /// the `dyn Ingredient.
     routes: Vec<Box<DynRoute<DB>>>,
 
-    // This is NOT indexed by ingredient index. It's just a vector to iterate over.
+    /// Vector if "mut routes". This vector is used to give callbacks
+    /// when new revisions are trigged. It is not indexed by ingredient
+    /// index as not every ingredient needs a callback; instead, you
+    /// just iterate over it and invoke each fn to get a `MutIngredient`.
     mut_routes: Vec<Box<DynMutRoute<DB>>>,
 }
 
 impl<DB: HasJars> Ingredients<DB> {
+    /// Construct an empty ingredients listing.
     pub(super) fn new() -> Self {
         Ingredients {
             routes: vec![],
@@ -63,7 +92,14 @@ impl<DB: HasJars> Ingredients<DB> {
     }
 
     /// As [`Self::push`] but for an ingredient that wants a callback whenever
-    /// a new revision is published. T+ Send + Synchese callbacks are used to clear out old data.
+    /// a new revision is published.
+    /// Many ingredients will, given an `&'db dyn Db` reference,
+    /// return a `&'db V` reference to one of their values.
+    /// This forces those values to live as long as the `&dyn Db` is valid.
+    /// This callback is invoked when a new revision begins.
+    /// It allows those values that were accessed to be freed.
+    /// We know it is safe to free them because, when a new revision starts,
+    /// we have an `&mut dyn Db`, and hence the `&dyn Db` lifetime must have ended.
     pub fn push_mut(
         &mut self,
         route: impl (Fn(&DB::Jars) -> &dyn Ingredient<DB>) + Send + Sync + 'static,
@@ -74,10 +110,14 @@ impl<DB: HasJars> Ingredients<DB> {
         index
     }
 
+    /// Given an ingredient index, return the "route"
+    /// (a function that, given a `Jars`, returns the ingredient).
     pub fn route(&self, index: IngredientIndex) -> &dyn Fn(&DB::Jars) -> &dyn Ingredient<DB> {
         &self.routes[index.as_usize()]
     }
 
+    /// Returns the "mutable routes" for the purposes of giving callbacks when a new revision is published.
+    /// Not all ingredients need callbacks, so this returns an iterator of just those that do.
     pub fn mut_routes(
         &self,
     ) -> impl Iterator<Item = &dyn Fn(&mut DB::Jars) -> &mut dyn MutIngredient<DB>> + '_ {

components/salsa-2022/src/runtime.rs

@@ -21,7 +21,6 @@ mod dependency_graph;
 pub mod local_state;
 mod shared_state;
 
-#[allow(dead_code)]
 pub struct Runtime {
     /// Our unique runtime id.
     id: RuntimeId,
@@ -100,7 +99,6 @@ impl Runtime {
         self.shared_state.empty_dependencies.clone()
     }
 
-    #[allow(dead_code)]
     pub fn snapshot(&self) -> Self {
         if self.local_state.query_in_progress() {
             panic!("it is not legal to `snapshot` during a query (see salsa-rs/salsa#80)");

components/salsa-2022/src/runtime/local_state.rs

@@ -56,7 +56,9 @@ impl QueryRevisions {
 /// Every input.
 #[derive(Debug, Clone)]
 pub struct QueryInputs {
-    /// Inputs that are fully known
+    /// Inputs that are fully known.
+    /// We track these even if there are unknown inputs so that the accumulator code
+    /// can walk all the inputs even for tracked functions that read untracked values.
     pub(crate) tracked: Arc<[DependencyIndex]>,
 
     /// Where there any *unknown* inputs?

components/salsa-2022/src/storage.rs

@@ -13,15 +13,33 @@ use crate::{Database, DatabaseKeyIndex, IngredientIndex};
 use super::routes::Ingredients;
 use super::{ParallelDatabase, Revision};
 
-#[allow(dead_code)]
+/// The "storage" struct stores all the data for the jars.
+/// It is shared between the main database and any active snapshots.
 pub struct Storage<DB: HasJars> {
+    /// Data shared across all databases.
     shared: Arc<Shared<DB>>,
+
+    /// The "ingredients" structure stores the information about how to find each ingredient in the database.
+    /// It allows us to take the [`IngredientIndex`] assigned to a particular ingredient
+    /// and get back a [`dyn Ingredient`][`Ingredient`] for the struct that stores its data.
     ingredients: Arc<Ingredients<DB>>,
+
+    /// The runtime for this particular salsa database handle.
+    /// Each handle gets its own runtime, but the runtimes have shared state between them.s
     runtime: Runtime,
 }
 
+/// Data shared between all threads.
+/// This is where the actual data for tracked functions, structs, inputs, etc lives,
+/// along with some coordination variables between treads.
 struct Shared<DB: HasJars> {
+    /// Contains the data for each jar in the database.
+    /// Each jar stores its own structs in there that ultimately contain ingredients
+    /// (types that implement the [`Ingredient`] trait, like [`crate::function::FunctionIngredient`]).
     jars: DB::Jars,
+
+    /// Conditional variable that is used to coordinate cancellation.
+    /// When the main thread writes to the database, it blocks until each of the snapshots can be cancelled.
     cvar: Condvar,
 }
 

components/salsa-2022/src/tracked_struct.rs

@@ -17,7 +17,14 @@ pub trait TrackedStructInDb<DB: ?Sized + Database> {
     fn database_key_index(self, db: &DB) -> DatabaseKeyIndex;
 }
 
-#[allow(dead_code)]
+/// Created for each tracked struct.
+/// This ingredient only stores the "id" fields.
+/// It is a kind of "dressed up" interner;
+/// the active query + values of id fields are hashed to create the tracked struct id.
+/// The value fields are stored in [`crate::function::FunctionIngredient`] instances keyed by the tracked struct id.
+/// Unlike normal interners, tracked struct indices can be deleted and reused aggressively:
+/// when a tracked function re-executes,
+/// any tracked structs that it created before but did not create this time can be deleted.
 pub struct TrackedStructIngredient<Id, Data>
 where
     Id: TrackedStructId,

salsa-2022-tests/tests/hello_world.rs

@@ -1,6 +1,5 @@
 //! Test that a `tracked` fn on a `salsa::input`
 //! compiles and executes successfully.
-#![allow(dead_code)]
 
 use salsa_2022_tests::{HasLogger, Logger};
 

salsa-2022-tests/tests/tracked_fn_on_tracked.rs

@@ -1,6 +1,5 @@
 //! Test that a `tracked` fn on a `salsa::input`
 //! compiles and executes successfully.
-#![allow(dead_code)]
 
 #[salsa::jar(db = Db)]
 struct Jar(MyInput, MyTracked, tracked_fn);

salsa-2022-tests/tests/tracked_fn_read_own_entity.rs

@@ -1,7 +1,5 @@
 //! Test that a `tracked` fn on a `salsa::input`
 //! compiles and executes successfully.
-#![allow(dead_code)]
-
 use salsa_2022_tests::{HasLogger, Logger};
 
 use expect_test::expect;