diff --git a/book/src/cycles.md b/book/src/cycles.md index 8222d9eaf..2215b8ff3 100644 --- a/book/src/cycles.md +++ b/book/src/cycles.md @@ -21,11 +21,13 @@ fn initial(_db: &dyn KnobsDatabase) -> u32 { } ``` -If `query` becomes the head of a cycle (that is, `query` is executing and on the active query stack, it calls `query2`, `query2` calls `query3`, and `query3` calls `query` again -- there could be any number of queries involved in the cycle), the `initial_fn` will be called to generate an "initial" value for `query` in the fixed-point computation. (The initial value should usually be the "bottom" value in the partial order.) All queries in the cycle will compute a provisional result based on this initial value for the cycle head. That is, `query3` will compute a provisional result using the initial value for `query`, `query2` will compute a provisional result using this provisional value for `query3`. When `cycle2` returns its provisional result back to `cycle`, `cycle` will observe that it has received a provisional result from its own cycle, and will call the `cycle_fn` (with the current value and the number of iterations that have occurred so far). The `cycle_fn` can return `salsa::CycleRecoveryAction::Iterate` to indicate that the cycle should iterate again, or `salsa::CycleRecoveryAction::Fallback(value)` to indicate that the cycle should stop iterating and fall back to the value provided. +If `query` becomes the head of a cycle (that is, `query` is executing and on the active query stack, it calls `query2`, `query2` calls `query3`, and `query3` calls `query` again -- there could be any number of queries involved in the cycle), the `initial_fn` will be called to generate an "initial" value for `query` in the fixed-point computation. (The initial value should usually be the "bottom" value in the partial order.) All queries in the cycle will compute a provisional result based on this initial value for the cycle head. That is, `query3` will compute a provisional result using the initial value for `query`, `query2` will compute a provisional result using this provisional value for `query3`. When `cycle2` returns its provisional result back to `cycle`, `cycle` will observe that it has received a provisional result from its own cycle, and will call the `cycle_fn` (with the current value and the number of iterations that have occurred so far). The `cycle_fn` can return `salsa::CycleRecoveryAction::Iterate` to indicate that the cycle should iterate again, or `salsa::CycleRecoveryAction::Fallback(value)` to indicate that fixpoint iteration should resume starting with the given value (which should be a value that will converge quickly). -If the `cycle_fn` continues to return `Iterate`, the cycle will iterate until it converges: that is, until two successive iterations produce the same result. +The cycle will iterate until it converges: that is, until two successive iterations produce the same result. -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. +If the `cycle_fn` returns `Fallback`, the cycle will still continue to iterate (using the given value as a new starting point), in order to verify that the fallback value results in a stable converged cycle. 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. + +If a cycle iterates more than 200 times, Salsa will panic rather than iterate forever. ## All potential cycle heads must set `cycle_fn` and `cycle_initial` diff --git a/src/event.rs b/src/event.rs index fbe8a784e..310c565f0 100644 --- a/src/event.rs +++ b/src/event.rs @@ -63,7 +63,6 @@ pub enum EventKind { /// The database-key for the cycle head. Implements `Debug`. database_key: DatabaseKeyIndex, iteration_count: IterationCount, - fell_back: bool, }, /// Indicates that `unwind_if_cancelled` was called and salsa will check if diff --git a/src/function/execute.rs b/src/function/execute.rs index d1651859e..723964851 100644 --- a/src/function/execute.rs +++ b/src/function/execute.rs @@ -134,7 +134,6 @@ where ) -> (C::Output<'db>, CompletedQuery) { let database_key_index = active_query.database_key_index; let mut iteration_count = IterationCount::initial(); - let mut fell_back = false; let zalsa_local = db.zalsa_local(); // Our provisional value from the previous iteration, when doing fixpoint iteration. @@ -192,16 +191,6 @@ where // If the new result is equal to the last provisional result, the cycle has // converged and we are done. if !C::values_equal(&new_value, last_provisional_value) { - if fell_back { - // We fell back to a value last iteration, but the fallback didn't result - // in convergence. We only have bad options here: continue iterating - // (ignoring the request to fall back), or forcibly use the fallback and - // leave the cycle in an inconsistent state (we'll be using a value for - // this query that it doesn't evaluate to, given its inputs). Maybe we'll - // have to go with the latter, but for now let's panic and see if real use - // cases need non-converging fallbacks. - panic!("{database_key_index:?}: execute: fallback did not converge"); - } // We are in a cycle that hasn't converged; ask the user's // cycle-recovery function what to do: match C::recover_from_cycle( @@ -216,10 +205,6 @@ where "{database_key_index:?}: execute: user cycle_fn says to fall back" ); new_value = fallback_value; - // We have to insert the fallback value for this query and then iterate - // one more time to fill in correct values for everything else in the - // cycle based on it; then we'll re-insert it as final value. - fell_back = true; } } // `iteration_count` can't overflow as we check it against `MAX_ITERATIONS` @@ -231,7 +216,6 @@ where Event::new(EventKind::WillIterateCycle { database_key: database_key_index, iteration_count, - fell_back, }) }); cycle_heads.update_iteration_count(database_key_index, iteration_count); diff --git a/tests/cycle.rs b/tests/cycle.rs index d226a9eb7..7a7e26a07 100644 --- a/tests/cycle.rs +++ b/tests/cycle.rs @@ -436,7 +436,7 @@ fn two_fallback_count() { /// /// Two-query cycle, falls back but fallback does not converge. #[test] -#[should_panic(expected = "fallback did not converge")] +#[should_panic(expected = "too many cycle iterations")] fn two_fallback_diverge() { let mut db = DbImpl::new(); let a_in = Inputs::new(&db, vec![]); @@ -1062,7 +1062,7 @@ fn cycle_sibling_interference() { "salsa_event(WillExecute { database_key: min_iterate(Id(0)) })", "salsa_event(WillExecute { database_key: min_iterate(Id(1)) })", "salsa_event(WillExecute { database_key: min_panic(Id(3)) })", - "salsa_event(WillIterateCycle { database_key: min_iterate(Id(0)), iteration_count: IterationCount(1), fell_back: false })", + "salsa_event(WillIterateCycle { database_key: min_iterate(Id(0)), iteration_count: IterationCount(1) })", "salsa_event(WillExecute { database_key: min_iterate(Id(1)) })", ]"#]]); } @@ -1093,7 +1093,7 @@ fn repeat_provisional_query() { "salsa_event(WillExecute { database_key: min_iterate(Id(0)) })", "salsa_event(WillExecute { database_key: min_panic(Id(1)) })", "salsa_event(WillExecute { database_key: min_panic(Id(2)) })", - "salsa_event(WillIterateCycle { database_key: min_iterate(Id(0)), iteration_count: IterationCount(1), fell_back: false })", + "salsa_event(WillIterateCycle { database_key: min_iterate(Id(0)), iteration_count: IterationCount(1) })", "salsa_event(WillExecute { database_key: min_panic(Id(1)) })", "salsa_event(WillExecute { database_key: min_panic(Id(2)) })", ]"#]]); @@ -1132,7 +1132,7 @@ fn repeat_provisional_query_incremental() { "salsa_event(WillExecute { database_key: min_panic(Id(2)) })", "salsa_event(WillExecute { database_key: min_panic(Id(1)) })", "salsa_event(WillExecute { database_key: min_iterate(Id(0)) })", - "salsa_event(WillIterateCycle { database_key: min_iterate(Id(0)), iteration_count: IterationCount(1), fell_back: false })", + "salsa_event(WillIterateCycle { database_key: min_iterate(Id(0)), iteration_count: IterationCount(1) })", "salsa_event(WillExecute { database_key: min_panic(Id(1)) })", "salsa_event(WillExecute { database_key: min_panic(Id(2)) })", ]"#]]); @@ -1248,13 +1248,13 @@ fn repeat_query_participating_in_cycle() { "salsa_event(WillExecute { database_key: query_c(Id(0)) })", "salsa_event(WillExecute { database_key: query_d(Id(0)) })", "salsa_event(WillExecute { database_key: query_hot(Id(0)) })", - "salsa_event(WillIterateCycle { database_key: head(Id(0)), iteration_count: IterationCount(1), fell_back: false })", + "salsa_event(WillIterateCycle { database_key: head(Id(0)), iteration_count: IterationCount(1) })", "salsa_event(WillExecute { database_key: query_a(Id(0)) })", "salsa_event(WillExecute { database_key: query_b(Id(0)) })", "salsa_event(WillExecute { database_key: query_c(Id(0)) })", "salsa_event(WillExecute { database_key: query_d(Id(0)) })", "salsa_event(WillExecute { database_key: query_hot(Id(0)) })", - "salsa_event(WillIterateCycle { database_key: head(Id(0)), iteration_count: IterationCount(2), fell_back: false })", + "salsa_event(WillIterateCycle { database_key: head(Id(0)), iteration_count: IterationCount(2) })", "salsa_event(WillExecute { database_key: query_a(Id(0)) })", "salsa_event(WillExecute { database_key: query_b(Id(0)) })", "salsa_event(WillExecute { database_key: query_c(Id(0)) })", @@ -1362,13 +1362,13 @@ fn repeat_query_participating_in_cycle2() { "salsa_event(WillExecute { database_key: query_b(Id(0)) })", "salsa_event(WillExecute { database_key: query_c(Id(0)) })", "salsa_event(WillExecute { database_key: query_d(Id(0)) })", - "salsa_event(WillIterateCycle { database_key: head(Id(0)), iteration_count: IterationCount(1), fell_back: false })", + "salsa_event(WillIterateCycle { database_key: head(Id(0)), iteration_count: IterationCount(1) })", "salsa_event(WillExecute { database_key: query_a(Id(0)) })", "salsa_event(WillExecute { database_key: query_hot(Id(0)) })", "salsa_event(WillExecute { database_key: query_b(Id(0)) })", "salsa_event(WillExecute { database_key: query_c(Id(0)) })", "salsa_event(WillExecute { database_key: query_d(Id(0)) })", - "salsa_event(WillIterateCycle { database_key: head(Id(0)), iteration_count: IterationCount(2), fell_back: false })", + "salsa_event(WillIterateCycle { database_key: head(Id(0)), iteration_count: IterationCount(2) })", "salsa_event(WillExecute { database_key: query_a(Id(0)) })", "salsa_event(WillExecute { database_key: query_hot(Id(0)) })", "salsa_event(WillExecute { database_key: query_b(Id(0)) })", diff --git a/tests/cycle_output.rs b/tests/cycle_output.rs index 27ba304e3..59b789aa4 100644 --- a/tests/cycle_output.rs +++ b/tests/cycle_output.rs @@ -195,13 +195,13 @@ fn revalidate_with_change_after_output_read() { "salsa_event(WillDiscardStaleOutput { execute_key: query_a(Id(0)), output_key: Output(Id(403)) })", "salsa_event(DidDiscard { key: Output(Id(403)) })", "salsa_event(DidDiscard { key: read_value(Id(403)) })", - "salsa_event(WillIterateCycle { database_key: query_b(Id(0)), iteration_count: IterationCount(1), fell_back: false })", + "salsa_event(WillIterateCycle { database_key: query_b(Id(0)), iteration_count: IterationCount(1) })", "salsa_event(WillExecute { database_key: query_a(Id(0)) })", "salsa_event(WillExecute { database_key: read_value(Id(401g1)) })", - "salsa_event(WillIterateCycle { database_key: query_b(Id(0)), iteration_count: IterationCount(2), fell_back: false })", + "salsa_event(WillIterateCycle { database_key: query_b(Id(0)), iteration_count: IterationCount(2) })", "salsa_event(WillExecute { database_key: query_a(Id(0)) })", "salsa_event(WillExecute { database_key: read_value(Id(402g1)) })", - "salsa_event(WillIterateCycle { database_key: query_b(Id(0)), iteration_count: IterationCount(3), fell_back: false })", + "salsa_event(WillIterateCycle { database_key: query_b(Id(0)), iteration_count: IterationCount(3) })", "salsa_event(WillExecute { database_key: query_a(Id(0)) })", "salsa_event(WillExecute { database_key: read_value(Id(403g1)) })", ]"#]]); diff --git a/tests/cycle_recovery_call_back_into_cycle.rs b/tests/cycle_recovery_call_back_into_cycle.rs index af7c10219..805a2be7b 100644 --- a/tests/cycle_recovery_call_back_into_cycle.rs +++ b/tests/cycle_recovery_call_back_into_cycle.rs @@ -37,7 +37,6 @@ fn converges() { } #[test] -#[should_panic(expected = "fallback did not converge")] fn diverges() { let db = DatabaseWithValue::new(3); diff --git a/tests/cycle_tracked.rs b/tests/cycle_tracked.rs index 27e52934d..154ba3370 100644 --- a/tests/cycle_tracked.rs +++ b/tests/cycle_tracked.rs @@ -165,7 +165,7 @@ fn main() { "WillExecute { database_key: cost_to_start(Id(401)) }", "WillCheckCancellation", "WillCheckCancellation", - "WillIterateCycle { database_key: cost_to_start(Id(403)), iteration_count: IterationCount(1), fell_back: false }", + "WillIterateCycle { database_key: cost_to_start(Id(403)), iteration_count: IterationCount(1) }", "WillCheckCancellation", "WillCheckCancellation", "WillCheckCancellation", @@ -307,9 +307,9 @@ fn test_cycle_with_fixpoint_structs() { "WillCheckCancellation", "WillExecute { database_key: create_tracked_in_cycle(Id(0)) }", "WillCheckCancellation", - "WillIterateCycle { database_key: create_tracked_in_cycle(Id(0)), iteration_count: IterationCount(1), fell_back: false }", + "WillIterateCycle { database_key: create_tracked_in_cycle(Id(0)), iteration_count: IterationCount(1) }", "WillCheckCancellation", - "WillIterateCycle { database_key: create_tracked_in_cycle(Id(0)), iteration_count: IterationCount(2), fell_back: false }", + "WillIterateCycle { database_key: create_tracked_in_cycle(Id(0)), iteration_count: IterationCount(2) }", "WillCheckCancellation", "WillDiscardStaleOutput { execute_key: create_tracked_in_cycle(Id(0)), output_key: IterationNode(Id(402)) }", "DidDiscard { key: IterationNode(Id(402)) }", diff --git a/tests/cycle_tracked_own_input.rs b/tests/cycle_tracked_own_input.rs index 17e8b815e..38218f1a7 100644 --- a/tests/cycle_tracked_own_input.rs +++ b/tests/cycle_tracked_own_input.rs @@ -117,7 +117,7 @@ fn main() { "WillExecute { database_key: infer_type_param(Id(400)) }", "WillCheckCancellation", "DidInternValue { key: Class(Id(c00)), revision: R2 }", - "WillIterateCycle { database_key: infer_class(Id(0)), iteration_count: IterationCount(1), fell_back: false }", + "WillIterateCycle { database_key: infer_class(Id(0)), iteration_count: IterationCount(1) }", "WillCheckCancellation", "WillExecute { database_key: infer_type_param(Id(400)) }", "WillCheckCancellation",