Allow immediately resolving cycles with a fallback instead of fixpoint

book/src/cycles.md

@@ -2,7 +2,11 @@
 
 By default, when Salsa detects a cycle in the computation graph, Salsa will panic with a message naming the "cycle head"; this is the query that was called while it was also on the active query stack, creating a cycle.
 
-Salsa also supports recovering from query cycles via fixed-point iteration. Fixed-point iteration is only usable if the queries which may be involved in a cycle are monotone and operate on a value domain which is a partial order with fixed height. Effectively, this means that the queries' output must always be "larger" than its input, and there must be some "maximum" or "top" value. This ensures that fixed-point iteration will converge to a value. (A typical case would be queries operating on types, which form a partial order with a "top" type.)
+Salsa supports three recovery modes: panicking (the default), fixpoint resolution and immediate fallback.
+
+## Fixpoint Resolution
+
+Fixed-point iteration is only usable if the queries which may be involved in a cycle are monotone and operate on a value domain which is a partial order with fixed height. Effectively, this means that the queries' output must always be "larger" than its input, and there must be some "maximum" or "top" value. This ensures that fixed-point iteration will converge to a value. (A typical case would be queries operating on types, which form a partial order with a "top" type.)
 
 In order to support fixed-point iteration for a query, provide the `cycle_fn` and `cycle_initial` arguments to `salsa::tracked`:
 
@@ -27,14 +31,38 @@ If the `cycle_fn` continues to return `Iterate`, the cycle will iterate until it
 
 If the `cycle_fn` returns `Fallback`, the cycle will iterate one last time and verify that the returned value is the same as the fallback value; that is, the fallback value results in a stable converged cycle. If not, Salsa will panic. It is not permitted to use a fallback value that does not converge, because this would leave the cycle in an unpredictable state, depending on the order of query execution.
 
-## All potential cycle heads must set `cycle_fn` and `cycle_initial`
+### All potential cycle heads must set `cycle_fn` and `cycle_initial`
 
 Consider a two-query cycle where `query_a` calls `query_b`, and `query_b` calls `query_a`. If `query_a` is called first, then it will become the "cycle head", but if `query_b` is called first, then `query_b` will be the cycle head. In order for a cycle to use fixed-point iteration instead of panicking, the cycle head must set `cycle_fn` and `cycle_initial`. This means that in order to be robust against varying query execution order, both `query_a` and `query_b` must set `cycle_fn` and `cycle_initial`.
 
-## Ensuring convergence
+### Ensuring convergence
 
 Fixed-point iteration is a powerful tool, but is also easy to misuse, potentially resulting in infinite iteration. To avoid this, ensure that all queries participating in fixpoint iteration are deterministic and monotone.
 
-## Calling Salsa queries from within `cycle_fn` or `cycle_initial`
+### Calling Salsa queries from within `cycle_fn` or `cycle_initial`
 
 It is permitted to call other Salsa queries from within the `cycle_fn` and `cycle_initial` functions. However, if these functions re-enter the same cycle, this can lead to unpredictable results. Take care which queries are called from within cycle-recovery functions, and avoid triggering further cycles.
+
+## Immediate Fallback
+
+This mode of cycle handling causes query calls that result in a cycle to immediately return with a fallback value.
+
+In order to support this fallback for a query, provide the `cycle_result` argument to `salsa::tracked`:
+
+```rust
+#[salsa::tracked(cycle_result=fallback)]
+fn query(db: &dyn salsa::Database) -> u32 {
+    // ...
+}
+
+fn fallback(_db: &dyn KnobsDatabase) -> u32 {
+    0
+}
+```
+
+### Observable execution order
+
+One problem with this fallback mode is that execution order / entry points become part of the query computation and can affect the results of queries containing cycles.
+This introduces a potential form on non-determinism depending on the query graph when multiple differing cycling queries are involved.
+Due to this, when an immediate fallback cycle occurs, salsa walks back the active query stacks to verify that the cycle does not occur within the context of another non-panic cycle query.
+In other words, it is only valid to immediate fallback cycle recover for a query if either all ancestors queries are panic cycle queries or if the cycle is immediate self-referential.

components/salsa-macros/src/accumulator.rs

@@ -40,6 +40,7 @@ impl AllowedOptions for Accumulator {
     const DB: bool = false;
     const CYCLE_FN: bool = false;
     const CYCLE_INITIAL: bool = false;
+    const CYCLE_RESULT: bool = false;
     const LRU: bool = false;
     const CONSTRUCTOR_NAME: bool = false;
     const ID: bool = false;

components/salsa-macros/src/input.rs

@@ -55,6 +55,8 @@ impl crate::options::AllowedOptions for InputStruct {
 
     const CYCLE_INITIAL: bool = false;
 
+    const CYCLE_RESULT: bool = false;
+
     const LRU: bool = false;
 
     const CONSTRUCTOR_NAME: bool = true;

components/salsa-macros/src/interned.rs

@@ -55,6 +55,8 @@ impl crate::options::AllowedOptions for InternedStruct {
 
     const CYCLE_INITIAL: bool = false;
 
+    const CYCLE_RESULT: bool = false;
+
     const LRU: bool = false;
 
     const CONSTRUCTOR_NAME: bool = true;

components/salsa-macros/src/options.rs

@@ -61,6 +61,11 @@ pub(crate) struct Options<A: AllowedOptions> {
     /// If this is `Some`, the value is the `<path>`.
     pub cycle_initial: Option<syn::Path>,
 
+    /// The `cycle_result = <path>` option is the result for non-fixpoint cycle.
+    ///
+    /// If this is `Some`, the value is the `<path>`.
+    pub cycle_result: Option<syn::Expr>,
+
     /// The `data = <ident>` option is used to define the name of the data type for an interned
     /// struct.
     ///
@@ -100,6 +105,7 @@ impl<A: AllowedOptions> Default for Options<A> {
             db_path: Default::default(),
             cycle_fn: Default::default(),
             cycle_initial: Default::default(),
+            cycle_result: Default::default(),
             data: Default::default(),
             constructor_name: Default::default(),
             phantom: Default::default(),
@@ -123,6 +129,7 @@ pub(crate) trait AllowedOptions {
     const DB: bool;
     const CYCLE_FN: bool;
     const CYCLE_INITIAL: bool;
+    const CYCLE_RESULT: bool;
     const LRU: bool;
     const CONSTRUCTOR_NAME: bool;
     const ID: bool;
@@ -274,6 +281,22 @@ impl<A: AllowedOptions> syn::parse::Parse for Options<A> {
                         "`cycle_initial` option not allowed here",
                     ));
                 }
+            } else if ident == "cycle_result" {
+                if A::CYCLE_RESULT {
+                    let _eq = Equals::parse(input)?;
+                    let expr = syn::Expr::parse(input)?;
+                    if let Some(old) = options.cycle_result.replace(expr) {
+                        return Err(syn::Error::new(
+                            old.span(),
+                            "option `cycle_result` provided twice",
+                        ));
+                    }
+                } else {
+                    return Err(syn::Error::new(
+                        ident.span(),
+                        "`cycle_result` option not allowed here",
+                    ));
+                }
             } else if ident == "data" {
                 if A::DATA {
                     let _eq = Equals::parse(input)?;

components/salsa-macros/src/tracked_fn.rs

@@ -48,6 +48,8 @@ impl crate::options::AllowedOptions for TrackedFn {
 
     const CYCLE_INITIAL: bool = true;
 
+    const CYCLE_RESULT: bool = true;
+
     const LRU: bool = true;
 
     const CONSTRUCTOR_NAME: bool = false;
@@ -201,25 +203,38 @@ impl Macro {
     fn cycle_recovery(&self) -> syn::Result<(TokenStream, TokenStream, TokenStream)> {
         // TODO should we ask the user to specify a struct that impls a trait with two methods,
         // rather than asking for two methods separately?
-        match (&self.args.cycle_fn, &self.args.cycle_initial) {
-            (Some(cycle_fn), Some(cycle_initial)) => Ok((
+        match (
+            &self.args.cycle_fn,
+            &self.args.cycle_initial,
+            &self.args.cycle_result,
+        ) {
+            (Some(cycle_fn), Some(cycle_initial), None) => Ok((
                 quote!((#cycle_fn)),
                 quote!((#cycle_initial)),
                 quote!(Fixpoint),
             )),
-            (None, None) => Ok((
+            (None, None, None) => Ok((
                 quote!((salsa::plumbing::unexpected_cycle_recovery!)),
                 quote!((salsa::plumbing::unexpected_cycle_initial!)),
                 quote!(Panic),
             )),
-            (Some(_), None) => Err(syn::Error::new_spanned(
+            (Some(_), None, None) => Err(syn::Error::new_spanned(
                 self.args.cycle_fn.as_ref().unwrap(),
                 "must provide `cycle_initial` along with `cycle_fn`",
             )),
-            (None, Some(_)) => Err(syn::Error::new_spanned(
+            (None, Some(_), None) => Err(syn::Error::new_spanned(
                 self.args.cycle_initial.as_ref().unwrap(),
                 "must provide `cycle_fn` along with `cycle_initial`",
             )),
+            (None, None, Some(cycle_result)) => Ok((
+                quote!((salsa::plumbing::unexpected_cycle_recovery!)),
+                quote!((#cycle_result)),
+                quote!(FallbackImmediate),
+            )),
+            (_, _, Some(_)) => Err(syn::Error::new_spanned(
+                self.args.cycle_initial.as_ref().unwrap(),
+                "must provide either both `cycle_fn` and `cycle_initial`, or only `cycle_result`",
+            )),
         }
     }
 

components/salsa-macros/src/tracked_struct.rs

@@ -50,6 +50,8 @@ impl crate::options::AllowedOptions for TrackedStruct {
 
     const CYCLE_INITIAL: bool = false;
 
+    const CYCLE_RESULT: bool = false;
+
     const LRU: bool = false;
 
     const CONSTRUCTOR_NAME: bool = true;

src/active_query.rs

@@ -5,7 +5,7 @@ use std::{mem, ops};
 use crate::accumulator::accumulated_map::{
     AccumulatedMap, AtomicInputAccumulatedValues, InputAccumulatedValues,
 };
-use crate::cycle::CycleHeads;
+use crate::cycle::{CycleHeads, CycleRecoveryStrategy};
 use crate::durability::Durability;
 use crate::hash::FxIndexSet;
 use crate::key::DatabaseKeyIndex;
@@ -63,6 +63,9 @@ pub(crate) struct ActiveQuery {
 
     /// If this query is a cycle head, iteration count of that cycle.
     iteration_count: u32,
+
+    /// The cycle strategy for this query.
+    cycle_strategy: CycleRecoveryStrategy,
 }
 
 impl ActiveQuery {
@@ -137,10 +140,18 @@ impl ActiveQuery {
     pub(super) fn iteration_count(&self) -> u32 {
         self.iteration_count
     }
+
+    pub(crate) fn cycle_strategy(&self) -> CycleRecoveryStrategy {
+        self.cycle_strategy
+    }
 }
 
 impl ActiveQuery {
-    fn new(database_key_index: DatabaseKeyIndex, iteration_count: u32) -> Self {
+    fn new(
+        database_key_index: DatabaseKeyIndex,
+        iteration_count: u32,
+        cycle_strategy: CycleRecoveryStrategy,
+    ) -> Self {
         ActiveQuery {
             database_key_index,
             durability: Durability::MAX,
@@ -153,6 +164,7 @@ impl ActiveQuery {
             accumulated_inputs: Default::default(),
             cycle_heads: Default::default(),
             iteration_count,
+            cycle_strategy,
         }
     }
 
@@ -169,6 +181,7 @@ impl ActiveQuery {
             accumulated_inputs,
             ref mut cycle_heads,
             iteration_count: _,
+            cycle_strategy: _,
         } = self;
 
         let edges = QueryEdges::new(input_outputs.drain(..));
@@ -210,6 +223,7 @@ impl ActiveQuery {
             accumulated_inputs: _,
             cycle_heads,
             iteration_count,
+            cycle_strategy: _,
         } = self;
         input_outputs.clear();
         disambiguator_map.clear();
@@ -219,7 +233,12 @@ impl ActiveQuery {
         *iteration_count = 0;
     }
 
-    fn reset_for(&mut self, new_database_key_index: DatabaseKeyIndex, new_iteration_count: u32) {
+    fn reset_for(
+        &mut self,
+        new_database_key_index: DatabaseKeyIndex,
+        new_iteration_count: u32,
+        new_cycle_strategy: CycleRecoveryStrategy,
+    ) {
         let Self {
             database_key_index,
             durability,
@@ -232,13 +251,15 @@ impl ActiveQuery {
             accumulated_inputs,
             cycle_heads,
             iteration_count,
+            cycle_strategy: is_cycle_result_query,
         } = self;
         *database_key_index = new_database_key_index;
         *durability = Durability::MAX;
         *changed_at = Revision::start();
         *untracked_read = false;
         *accumulated_inputs = Default::default();
         *iteration_count = new_iteration_count;
+        *is_cycle_result_query = new_cycle_strategy;
         debug_assert!(
             input_outputs.is_empty(),
             "`ActiveQuery::clear` or `ActiveQuery::into_revisions` should've been called"
@@ -304,12 +325,16 @@ impl QueryStack {
         &mut self,
         database_key_index: DatabaseKeyIndex,
         iteration_count: u32,
+        cycle_strategy: CycleRecoveryStrategy,
     ) {
         if self.len < self.stack.len() {
-            self.stack[self.len].reset_for(database_key_index, iteration_count);
+            self.stack[self.len].reset_for(database_key_index, iteration_count, cycle_strategy);
         } else {
-            self.stack
-                .push(ActiveQuery::new(database_key_index, iteration_count));
+            self.stack.push(ActiveQuery::new(
+                database_key_index,
+                iteration_count,
+                cycle_strategy,
+            ));
         }
         self.len += 1;
     }

src/cycle.rs

@@ -83,6 +83,10 @@ pub enum CycleRecoveryStrategy {
     /// This choice is computed by the query's `cycle_recovery`
     /// function and initial value.
     Fixpoint,
+
+    /// Recovers from cycles by falling back to a sentinel value
+    /// for cycle invocations.
+    FallbackImmediate,
 }
 
 /// A "cycle head" is the query at which we encounter a cycle; that is, if A -> B -> C -> A, then A

src/function/execute.rs

@@ -72,7 +72,7 @@ where
                     // initial provisional value from there.
                     let memo =
                         self.get_memo_from_table_for(zalsa, id, memo_ingredient_index)
-                        .unwrap_or_else(|| panic!("{database_key_index:#?} is a cycle head, but no provisional memo found"));
+                        .unwrap_or_else(|| unreachable!("{database_key_index:#?} is a cycle head, but no provisional memo found"));
                     debug_assert!(memo.may_be_provisional());
                     memo.value.as_ref()
                 };
@@ -140,9 +140,11 @@ where
                         memo_ingredient_index,
                     ));
 
-                    active_query = db
-                        .zalsa_local()
-                        .push_query(database_key_index, iteration_count);
+                    active_query = db.zalsa_local().push_query(
+                        database_key_index,
+                        iteration_count,
+                        C::CYCLE_STRATEGY,
+                    );
 
                     continue;
                 }