Skip to content

Commit 7f0d2a0

Browse files
committed
Allow fallback to take longer than one iteration to converge
1 parent a0e7a06 commit 7f0d2a0

File tree

8 files changed

+20
-36
lines changed

8 files changed

+20
-36
lines changed

book/src/cycles.md

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,13 @@ fn initial(_db: &dyn KnobsDatabase) -> u32 {
2121
}
2222
```
2323

24-
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.
24+
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).
2525

26-
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.
26+
The cycle will iterate until it converges: that is, until two successive iterations produce the same result.
2727

28-
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.
28+
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.
29+
30+
If a cycle iterates more than 200 times, Salsa will panic rather than iterate forever.
2931

3032
## All potential cycle heads must set `cycle_fn` and `cycle_initial`
3133

src/event.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,6 @@ pub enum EventKind {
6363
/// The database-key for the cycle head. Implements `Debug`.
6464
database_key: DatabaseKeyIndex,
6565
iteration_count: IterationCount,
66-
fell_back: bool,
6766
},
6867

6968
/// Indicates that `unwind_if_cancelled` was called and salsa will check if

src/function/execute.rs

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,6 @@ where
134134
) -> (C::Output<'db>, CompletedQuery) {
135135
let database_key_index = active_query.database_key_index;
136136
let mut iteration_count = IterationCount::initial();
137-
let mut fell_back = false;
138137
let zalsa_local = db.zalsa_local();
139138

140139
// Our provisional value from the previous iteration, when doing fixpoint iteration.
@@ -192,16 +191,6 @@ where
192191
// If the new result is equal to the last provisional result, the cycle has
193192
// converged and we are done.
194193
if !C::values_equal(&new_value, last_provisional_value) {
195-
if fell_back {
196-
// We fell back to a value last iteration, but the fallback didn't result
197-
// in convergence. We only have bad options here: continue iterating
198-
// (ignoring the request to fall back), or forcibly use the fallback and
199-
// leave the cycle in an inconsistent state (we'll be using a value for
200-
// this query that it doesn't evaluate to, given its inputs). Maybe we'll
201-
// have to go with the latter, but for now let's panic and see if real use
202-
// cases need non-converging fallbacks.
203-
panic!("{database_key_index:?}: execute: fallback did not converge");
204-
}
205194
// We are in a cycle that hasn't converged; ask the user's
206195
// cycle-recovery function what to do:
207196
match C::recover_from_cycle(
@@ -216,10 +205,6 @@ where
216205
"{database_key_index:?}: execute: user cycle_fn says to fall back"
217206
);
218207
new_value = fallback_value;
219-
// We have to insert the fallback value for this query and then iterate
220-
// one more time to fill in correct values for everything else in the
221-
// cycle based on it; then we'll re-insert it as final value.
222-
fell_back = true;
223208
}
224209
}
225210
// `iteration_count` can't overflow as we check it against `MAX_ITERATIONS`
@@ -231,7 +216,6 @@ where
231216
Event::new(EventKind::WillIterateCycle {
232217
database_key: database_key_index,
233218
iteration_count,
234-
fell_back,
235219
})
236220
});
237221
cycle_heads.update_iteration_count(database_key_index, iteration_count);

tests/cycle.rs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -436,7 +436,7 @@ fn two_fallback_count() {
436436
///
437437
/// Two-query cycle, falls back but fallback does not converge.
438438
#[test]
439-
#[should_panic(expected = "fallback did not converge")]
439+
#[should_panic(expected = "too many cycle iterations")]
440440
fn two_fallback_diverge() {
441441
let mut db = DbImpl::new();
442442
let a_in = Inputs::new(&db, vec![]);
@@ -1062,7 +1062,7 @@ fn cycle_sibling_interference() {
10621062
"salsa_event(WillExecute { database_key: min_iterate(Id(0)) })",
10631063
"salsa_event(WillExecute { database_key: min_iterate(Id(1)) })",
10641064
"salsa_event(WillExecute { database_key: min_panic(Id(3)) })",
1065-
"salsa_event(WillIterateCycle { database_key: min_iterate(Id(0)), iteration_count: IterationCount(1), fell_back: false })",
1065+
"salsa_event(WillIterateCycle { database_key: min_iterate(Id(0)), iteration_count: IterationCount(1) })",
10661066
"salsa_event(WillExecute { database_key: min_iterate(Id(1)) })",
10671067
]"#]]);
10681068
}
@@ -1093,7 +1093,7 @@ fn repeat_provisional_query() {
10931093
"salsa_event(WillExecute { database_key: min_iterate(Id(0)) })",
10941094
"salsa_event(WillExecute { database_key: min_panic(Id(1)) })",
10951095
"salsa_event(WillExecute { database_key: min_panic(Id(2)) })",
1096-
"salsa_event(WillIterateCycle { database_key: min_iterate(Id(0)), iteration_count: IterationCount(1), fell_back: false })",
1096+
"salsa_event(WillIterateCycle { database_key: min_iterate(Id(0)), iteration_count: IterationCount(1) })",
10971097
"salsa_event(WillExecute { database_key: min_panic(Id(1)) })",
10981098
"salsa_event(WillExecute { database_key: min_panic(Id(2)) })",
10991099
]"#]]);
@@ -1132,7 +1132,7 @@ fn repeat_provisional_query_incremental() {
11321132
"salsa_event(WillExecute { database_key: min_panic(Id(2)) })",
11331133
"salsa_event(WillExecute { database_key: min_panic(Id(1)) })",
11341134
"salsa_event(WillExecute { database_key: min_iterate(Id(0)) })",
1135-
"salsa_event(WillIterateCycle { database_key: min_iterate(Id(0)), iteration_count: IterationCount(1), fell_back: false })",
1135+
"salsa_event(WillIterateCycle { database_key: min_iterate(Id(0)), iteration_count: IterationCount(1) })",
11361136
"salsa_event(WillExecute { database_key: min_panic(Id(1)) })",
11371137
"salsa_event(WillExecute { database_key: min_panic(Id(2)) })",
11381138
]"#]]);
@@ -1248,13 +1248,13 @@ fn repeat_query_participating_in_cycle() {
12481248
"salsa_event(WillExecute { database_key: query_c(Id(0)) })",
12491249
"salsa_event(WillExecute { database_key: query_d(Id(0)) })",
12501250
"salsa_event(WillExecute { database_key: query_hot(Id(0)) })",
1251-
"salsa_event(WillIterateCycle { database_key: head(Id(0)), iteration_count: IterationCount(1), fell_back: false })",
1251+
"salsa_event(WillIterateCycle { database_key: head(Id(0)), iteration_count: IterationCount(1) })",
12521252
"salsa_event(WillExecute { database_key: query_a(Id(0)) })",
12531253
"salsa_event(WillExecute { database_key: query_b(Id(0)) })",
12541254
"salsa_event(WillExecute { database_key: query_c(Id(0)) })",
12551255
"salsa_event(WillExecute { database_key: query_d(Id(0)) })",
12561256
"salsa_event(WillExecute { database_key: query_hot(Id(0)) })",
1257-
"salsa_event(WillIterateCycle { database_key: head(Id(0)), iteration_count: IterationCount(2), fell_back: false })",
1257+
"salsa_event(WillIterateCycle { database_key: head(Id(0)), iteration_count: IterationCount(2) })",
12581258
"salsa_event(WillExecute { database_key: query_a(Id(0)) })",
12591259
"salsa_event(WillExecute { database_key: query_b(Id(0)) })",
12601260
"salsa_event(WillExecute { database_key: query_c(Id(0)) })",
@@ -1362,13 +1362,13 @@ fn repeat_query_participating_in_cycle2() {
13621362
"salsa_event(WillExecute { database_key: query_b(Id(0)) })",
13631363
"salsa_event(WillExecute { database_key: query_c(Id(0)) })",
13641364
"salsa_event(WillExecute { database_key: query_d(Id(0)) })",
1365-
"salsa_event(WillIterateCycle { database_key: head(Id(0)), iteration_count: IterationCount(1), fell_back: false })",
1365+
"salsa_event(WillIterateCycle { database_key: head(Id(0)), iteration_count: IterationCount(1) })",
13661366
"salsa_event(WillExecute { database_key: query_a(Id(0)) })",
13671367
"salsa_event(WillExecute { database_key: query_hot(Id(0)) })",
13681368
"salsa_event(WillExecute { database_key: query_b(Id(0)) })",
13691369
"salsa_event(WillExecute { database_key: query_c(Id(0)) })",
13701370
"salsa_event(WillExecute { database_key: query_d(Id(0)) })",
1371-
"salsa_event(WillIterateCycle { database_key: head(Id(0)), iteration_count: IterationCount(2), fell_back: false })",
1371+
"salsa_event(WillIterateCycle { database_key: head(Id(0)), iteration_count: IterationCount(2) })",
13721372
"salsa_event(WillExecute { database_key: query_a(Id(0)) })",
13731373
"salsa_event(WillExecute { database_key: query_hot(Id(0)) })",
13741374
"salsa_event(WillExecute { database_key: query_b(Id(0)) })",

tests/cycle_output.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -195,13 +195,13 @@ fn revalidate_with_change_after_output_read() {
195195
"salsa_event(WillDiscardStaleOutput { execute_key: query_a(Id(0)), output_key: Output(Id(403)) })",
196196
"salsa_event(DidDiscard { key: Output(Id(403)) })",
197197
"salsa_event(DidDiscard { key: read_value(Id(403)) })",
198-
"salsa_event(WillIterateCycle { database_key: query_b(Id(0)), iteration_count: IterationCount(1), fell_back: false })",
198+
"salsa_event(WillIterateCycle { database_key: query_b(Id(0)), iteration_count: IterationCount(1) })",
199199
"salsa_event(WillExecute { database_key: query_a(Id(0)) })",
200200
"salsa_event(WillExecute { database_key: read_value(Id(401g1)) })",
201-
"salsa_event(WillIterateCycle { database_key: query_b(Id(0)), iteration_count: IterationCount(2), fell_back: false })",
201+
"salsa_event(WillIterateCycle { database_key: query_b(Id(0)), iteration_count: IterationCount(2) })",
202202
"salsa_event(WillExecute { database_key: query_a(Id(0)) })",
203203
"salsa_event(WillExecute { database_key: read_value(Id(402g1)) })",
204-
"salsa_event(WillIterateCycle { database_key: query_b(Id(0)), iteration_count: IterationCount(3), fell_back: false })",
204+
"salsa_event(WillIterateCycle { database_key: query_b(Id(0)), iteration_count: IterationCount(3) })",
205205
"salsa_event(WillExecute { database_key: query_a(Id(0)) })",
206206
"salsa_event(WillExecute { database_key: read_value(Id(403g1)) })",
207207
]"#]]);

tests/cycle_recovery_call_back_into_cycle.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@ fn converges() {
3737
}
3838

3939
#[test]
40-
#[should_panic(expected = "fallback did not converge")]
4140
fn diverges() {
4241
let db = DatabaseWithValue::new(3);
4342

tests/cycle_tracked.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ fn main() {
165165
"WillExecute { database_key: cost_to_start(Id(401)) }",
166166
"WillCheckCancellation",
167167
"WillCheckCancellation",
168-
"WillIterateCycle { database_key: cost_to_start(Id(403)), iteration_count: IterationCount(1), fell_back: false }",
168+
"WillIterateCycle { database_key: cost_to_start(Id(403)), iteration_count: IterationCount(1) }",
169169
"WillCheckCancellation",
170170
"WillCheckCancellation",
171171
"WillCheckCancellation",
@@ -307,9 +307,9 @@ fn test_cycle_with_fixpoint_structs() {
307307
"WillCheckCancellation",
308308
"WillExecute { database_key: create_tracked_in_cycle(Id(0)) }",
309309
"WillCheckCancellation",
310-
"WillIterateCycle { database_key: create_tracked_in_cycle(Id(0)), iteration_count: IterationCount(1), fell_back: false }",
310+
"WillIterateCycle { database_key: create_tracked_in_cycle(Id(0)), iteration_count: IterationCount(1) }",
311311
"WillCheckCancellation",
312-
"WillIterateCycle { database_key: create_tracked_in_cycle(Id(0)), iteration_count: IterationCount(2), fell_back: false }",
312+
"WillIterateCycle { database_key: create_tracked_in_cycle(Id(0)), iteration_count: IterationCount(2) }",
313313
"WillCheckCancellation",
314314
"WillDiscardStaleOutput { execute_key: create_tracked_in_cycle(Id(0)), output_key: IterationNode(Id(402)) }",
315315
"DidDiscard { key: IterationNode(Id(402)) }",

tests/cycle_tracked_own_input.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ fn main() {
117117
"WillExecute { database_key: infer_type_param(Id(400)) }",
118118
"WillCheckCancellation",
119119
"DidInternValue { key: Class(Id(c00)), revision: R2 }",
120-
"WillIterateCycle { database_key: infer_class(Id(0)), iteration_count: IterationCount(1), fell_back: false }",
120+
"WillIterateCycle { database_key: infer_class(Id(0)), iteration_count: IterationCount(1) }",
121121
"WillCheckCancellation",
122122
"WillExecute { database_key: infer_type_param(Id(400)) }",
123123
"WillCheckCancellation",

0 commit comments

Comments
 (0)