From 04b70f54e3c59184b8c2710bac4de7fcc54592c1 Mon Sep 17 00:00:00 2001 From: XFFXFF <1247714429@qq.com> Date: Sun, 21 Aug 2022 10:02:45 +0800 Subject: [PATCH 1/4] lru can be changed at runtime --- .../salsa-2022-macros/src/tracked_fn.rs | 36 +++++++++++++++ salsa-2022-tests/tests/lru.rs | 44 +++++++++++++++++++ 2 files changed, 80 insertions(+) diff --git a/components/salsa-2022-macros/src/tracked_fn.rs b/components/salsa-2022-macros/src/tracked_fn.rs index 4ce59b3d..8854d2c6 100644 --- a/components/salsa-2022-macros/src/tracked_fn.rs +++ b/components/salsa-2022-macros/src/tracked_fn.rs @@ -288,6 +288,7 @@ fn wrapper_fns( let accumulated_fn = accumulated_fn(args, item_fn, config_ty)?; let setter_fn = setter_fn(args, item_fn, config_ty)?; let specify_fn = specify_fn(args, item_fn, config_ty)?.map(|f| quote! { #f }); + let set_lru_fn = set_lru_capacity_fn(args, config_ty)?.map(|f| quote! { #f }); let setter_impl: syn::ItemImpl = parse_quote! { impl #config_ty { @@ -300,6 +301,8 @@ fn wrapper_fns( #[allow(dead_code, clippy::needless_lifetimes)] #accumulated_fn + #set_lru_fn + #specify_fn } }; @@ -415,6 +418,39 @@ fn setter_fn( }) } +/// Create a `set_lru_capacity` associated function that can be used to change LRU +/// capacity at runtime. +/// Note that this function is only generated if the tracked function has the lru option set. +/// +/// # Examples +/// +/// ```rust +/// #[salsa::tracked(lru=32)] +/// fn my_tracked_fn(db: &dyn crate::Db, ...) { } +/// +/// my_tracked_fn::set_lru_capacity(16) +/// ``` +fn set_lru_capacity_fn( + args: &Args, + config_ty: &syn::Type, +) -> syn::Result> { + if args.lru.is_none() { + return Ok(None); + } + + let jar_ty = args.jar_ty(); + let lru_fn = parse_quote! { + #[allow(dead_code, clippy::needless_lifetimes)] + fn set_lru_capacity(__db: &salsa::function::DynDb, __value: usize) { + let (__jar, __runtime) = <_ as salsa::storage::HasJar<#jar_ty>>::jar(__db); + let __ingredients = + <_ as salsa::storage::HasIngredientsFor<#config_ty>>::ingredient(__jar); + __ingredients.function.set_capacity(__value); + } + }; + Ok(Some(lru_fn)) +} + fn specify_fn( args: &Args, item_fn: &syn::ItemFn, diff --git a/salsa-2022-tests/tests/lru.rs b/salsa-2022-tests/tests/lru.rs index 21d2ecb4..59d6fcf1 100644 --- a/salsa-2022-tests/tests/lru.rs +++ b/salsa-2022-tests/tests/lru.rs @@ -103,3 +103,47 @@ fn lru_doesnt_break_volatile_queries() { } } } + +#[test] +fn lru_can_be_changed_at_runtime() { + let mut db = Database::default(); + assert_eq!(load_n_potatoes(), 0); + + for i in 0..128u32 { + let input = MyInput::new(&mut db, i); + let p = get_hot_potato(&db, input); + assert_eq!(p.0, i) + } + + // Create a new input to change the revision, and trigger the GC + MyInput::new(&mut db, 0); + assert_eq!(load_n_potatoes(), 32); + + get_hot_potato::set_lru_capacity(&db, 64); + assert_eq!(load_n_potatoes(), 32); + for i in 0..128u32 { + let input = MyInput::new(&mut db, i); + let p = get_hot_potato(&db, input); + assert_eq!(p.0, i) + } + + // Create a new input to change the revision, and trigger the GC + MyInput::new(&mut db, 0); + assert_eq!(load_n_potatoes(), 64); + + // Special case: setting capacity to zero disables LRU + get_hot_potato::set_lru_capacity(&db, 0); + assert_eq!(load_n_potatoes(), 64); + for i in 0..128u32 { + let input = MyInput::new(&mut db, i); + let p = get_hot_potato(&db, input); + assert_eq!(p.0, i) + } + + // Create a new input to change the revision, and trigger the GC + MyInput::new(&mut db, 0); + assert_eq!(load_n_potatoes(), 192); + + drop(db); + assert_eq!(load_n_potatoes(), 0); +} From 6645774d5592854980f2475dd953b3b846f2ece6 Mon Sep 17 00:00:00 2001 From: XFFXFF <1247714429@qq.com> Date: Sun, 21 Aug 2022 10:31:46 +0800 Subject: [PATCH 2/4] port another test about lru --- salsa-2022-tests/src/lib.rs | 8 ++++++ salsa-2022-tests/tests/lru.rs | 46 +++++++++++++++++++++++++++++++++-- 2 files changed, 52 insertions(+), 2 deletions(-) diff --git a/salsa-2022-tests/src/lib.rs b/salsa-2022-tests/src/lib.rs index beb78d28..affbcc4e 100644 --- a/salsa-2022-tests/src/lib.rs +++ b/salsa-2022-tests/src/lib.rs @@ -21,4 +21,12 @@ pub trait HasLogger { let logs = std::mem::replace(&mut *self.logger().logs.lock().unwrap(), vec![]); expected.assert_eq(&format!("{:#?}", logs)); } + + /// Asserts the length of the logs, + /// clearing the logged events. This takes `&mut self` because + /// it is meant to be run from outside any tracked functions. + fn assert_logs_len(&mut self, expected: usize) { + let logs = std::mem::replace(&mut *self.logger().logs.lock().unwrap(), vec![]); + assert_eq!(logs.len(), expected); + } } diff --git a/salsa-2022-tests/tests/lru.rs b/salsa-2022-tests/tests/lru.rs index 59d6fcf1..9c017437 100644 --- a/salsa-2022-tests/tests/lru.rs +++ b/salsa-2022-tests/tests/lru.rs @@ -6,12 +6,13 @@ use std::sync::{ Arc, }; +use salsa_2022_tests::{HasLogger, Logger}; use test_log::test; #[salsa::jar(db = Db)] -struct Jar(MyInput, get_hot_potato, get_volatile); +struct Jar(MyInput, get_hot_potato, get_hot_potato2, get_volatile); -trait Db: salsa::DbWithJar {} +trait Db: salsa::DbWithJar + HasLogger {} #[derive(Debug, PartialEq, Eq)] struct HotPotato(u32); @@ -40,9 +41,16 @@ struct MyInput { #[salsa::tracked(jar = Jar, lru = 32)] fn get_hot_potato(db: &dyn Db, input: MyInput) -> Arc { + db.push_log(format!("get_hot_potato({:?})", input.field(db))); Arc::new(HotPotato::new(input.field(db))) } +#[salsa::tracked(jar = Jar)] +fn get_hot_potato2(db: &dyn Db, input: MyInput) -> u32 { + db.push_log(format!("get_hot_potato2({:?})", input.field(db))); + get_hot_potato(db, input).0 +} + #[salsa::tracked(jar = Jar, lru = 32)] fn get_volatile(db: &dyn Db, _input: MyInput) -> usize { static COUNTER: AtomicUsize = AtomicUsize::new(0); @@ -54,6 +62,7 @@ fn get_volatile(db: &dyn Db, _input: MyInput) -> usize { #[derive(Default)] struct Database { storage: salsa::Storage, + logger: Logger, } impl salsa::Database for Database { @@ -64,6 +73,12 @@ impl salsa::Database for Database { impl Db for Database {} +impl HasLogger for Database { + fn logger(&self) -> &Logger { + &self.logger + } +} + fn load_n_potatoes() -> usize { N_POTATOES.with(|n| n.load(Ordering::SeqCst)) } @@ -147,3 +162,30 @@ fn lru_can_be_changed_at_runtime() { drop(db); assert_eq!(load_n_potatoes(), 0); } + +#[test] +fn lru_keeps_dependency_info() { + let mut db = Database::default(); + let capacity = 32; + + // Invoke `get_hot_potato2` 33 times. This will (in turn) invoke + // `get_hot_potato`, which will trigger LRU after 32 executions. + let inputs: Vec = (0..(capacity + 1)) + .map(|i| MyInput::new(&mut db, i as u32)) + .collect(); + + for (i, input) in inputs.iter().enumerate() { + let x = get_hot_potato2(&db, *input); + assert_eq!(x as usize, i); + } + + // We want to test that calls to `get_hot_potato2` are still considered + // clean. Check that no new executions occur as we go here. + db.assert_logs_len((capacity + 1) * 2); + + // calling `get_hot_potato2(0)` has to check that `get_hot_potato(0)` is still valid; + // even though we've evicted it (LRU), we find that it is still good + let p = get_hot_potato2(&db, *inputs.first().unwrap()); + assert_eq!(p, 0); + db.assert_logs_len(0); +} From 31a4c68fc74f4b7edbb7379564c2533463ae34bd Mon Sep 17 00:00:00 2001 From: XFFXFF <1247714429@qq.com> Date: Sun, 21 Aug 2022 14:11:26 +0800 Subject: [PATCH 3/4] fix typos: deponds -> depends --- ...ruct_changes_but_fn_depends_on_field_y.rs} | 30 +++++++++---------- ...nput_changes_but_fn_depends_on_field_y.rs} | 28 ++++++++--------- 2 files changed, 29 insertions(+), 29 deletions(-) rename salsa-2022-tests/tests/{expect_reuse_field_x_of_a_tracked_struct_changes_but_fn_deponds_on_field_y.rs => expect_reuse_field_x_of_a_tracked_struct_changes_but_fn_depends_on_field_y.rs} (69%) rename salsa-2022-tests/tests/{expect_reuse_field_x_of_an_input_changes_but_fn_deponds_on_field_y.rs => expect_reuse_field_x_of_an_input_changes_but_fn_depends_on_field_y.rs} (64%) diff --git a/salsa-2022-tests/tests/expect_reuse_field_x_of_a_tracked_struct_changes_but_fn_deponds_on_field_y.rs b/salsa-2022-tests/tests/expect_reuse_field_x_of_a_tracked_struct_changes_but_fn_depends_on_field_y.rs similarity index 69% rename from salsa-2022-tests/tests/expect_reuse_field_x_of_a_tracked_struct_changes_but_fn_deponds_on_field_y.rs rename to salsa-2022-tests/tests/expect_reuse_field_x_of_a_tracked_struct_changes_but_fn_depends_on_field_y.rs index 2a9633db..86cf517c 100644 --- a/salsa-2022-tests/tests/expect_reuse_field_x_of_a_tracked_struct_changes_but_fn_deponds_on_field_y.rs +++ b/salsa-2022-tests/tests/expect_reuse_field_x_of_a_tracked_struct_changes_but_fn_depends_on_field_y.rs @@ -11,8 +11,8 @@ use expect_test::expect; struct Jar( MyInput, MyTracked, - final_result_deponds_on_x, - final_result_deponds_on_y, + final_result_depends_on_x, + final_result_depends_on_y, intermediate_result, ); @@ -24,14 +24,14 @@ struct MyInput { } #[salsa::tracked(jar = Jar)] -fn final_result_deponds_on_x(db: &dyn Db, input: MyInput) -> u32 { - db.push_log(format!("final_result_deponds_on_x({:?})", input)); +fn final_result_depends_on_x(db: &dyn Db, input: MyInput) -> u32 { + db.push_log(format!("final_result_depends_on_x({:?})", input)); intermediate_result(db, input).x(db) * 2 } #[salsa::tracked(jar = Jar)] -fn final_result_deponds_on_y(db: &dyn Db, input: MyInput) -> u32 { - db.push_log(format!("final_result_deponds_on_y({:?})", input)); +fn final_result_depends_on_y(db: &dyn Db, input: MyInput) -> u32 { + db.push_log(format!("final_result_depends_on_y({:?})", input)); intermediate_result(db, input).y(db) * 2 } @@ -71,39 +71,39 @@ impl HasLogger for Database { fn execute() { // x = (input.field + 1) / 2 // y = input.field / 2 - // final_result_deponds_on_x = x * 2 = (input.field + 1) / 2 * 2 - // final_result_deponds_on_y = y * 2 = input.field / 2 * 2 + // final_result_depends_on_x = x * 2 = (input.field + 1) / 2 * 2 + // final_result_depends_on_y = y * 2 = input.field / 2 * 2 let mut db = Database::default(); // intermediate results: // x = (22 + 1) / 2 = 11 // y = 22 / 2 = 11 let input = MyInput::new(&mut db, 22); - assert_eq!(final_result_deponds_on_x(&db, input), 22); + assert_eq!(final_result_depends_on_x(&db, input), 22); db.assert_logs(expect![[r#" [ - "final_result_deponds_on_x(MyInput(Id { value: 1 }))", + "final_result_depends_on_x(MyInput(Id { value: 1 }))", ]"#]]); - assert_eq!(final_result_deponds_on_y(&db, input), 22); + assert_eq!(final_result_depends_on_y(&db, input), 22); db.assert_logs(expect![[r#" [ - "final_result_deponds_on_y(MyInput(Id { value: 1 }))", + "final_result_depends_on_y(MyInput(Id { value: 1 }))", ]"#]]); input.set_field(&mut db, 23); // x = (23 + 1) / 2 = 12 // Intermediate result x changes, so final result depends on x // needs to be recomputed; - assert_eq!(final_result_deponds_on_x(&db, input), 24); + assert_eq!(final_result_depends_on_x(&db, input), 24); db.assert_logs(expect![[r#" [ - "final_result_deponds_on_x(MyInput(Id { value: 1 }))", + "final_result_depends_on_x(MyInput(Id { value: 1 }))", ]"#]]); // y = 23 / 2 = 11 // Intermediate result y is the same, so final result depends on y // does not need to be recomputed; - assert_eq!(final_result_deponds_on_y(&db, input), 22); + assert_eq!(final_result_depends_on_y(&db, input), 22); db.assert_logs(expect!["[]"]); } diff --git a/salsa-2022-tests/tests/expect_reuse_field_x_of_an_input_changes_but_fn_deponds_on_field_y.rs b/salsa-2022-tests/tests/expect_reuse_field_x_of_an_input_changes_but_fn_depends_on_field_y.rs similarity index 64% rename from salsa-2022-tests/tests/expect_reuse_field_x_of_an_input_changes_but_fn_deponds_on_field_y.rs rename to salsa-2022-tests/tests/expect_reuse_field_x_of_an_input_changes_but_fn_depends_on_field_y.rs index 5efd337f..cb1331c7 100644 --- a/salsa-2022-tests/tests/expect_reuse_field_x_of_an_input_changes_but_fn_deponds_on_field_y.rs +++ b/salsa-2022-tests/tests/expect_reuse_field_x_of_an_input_changes_but_fn_depends_on_field_y.rs @@ -8,7 +8,7 @@ use salsa_2022_tests::{HasLogger, Logger}; use expect_test::expect; #[salsa::jar(db = Db)] -struct Jar(MyInput, result_deponds_on_x, result_deponds_on_y); +struct Jar(MyInput, result_depends_on_x, result_depends_on_y); trait Db: salsa::DbWithJar + HasLogger {} @@ -19,14 +19,14 @@ struct MyInput { } #[salsa::tracked(jar = Jar)] -fn result_deponds_on_x(db: &dyn Db, input: MyInput) -> u32 { - db.push_log(format!("result_deponds_on_x({:?})", input)); +fn result_depends_on_x(db: &dyn Db, input: MyInput) -> u32 { + db.push_log(format!("result_depends_on_x({:?})", input)); input.x(db) + 1 } #[salsa::tracked(jar = Jar)] -fn result_deponds_on_y(db: &dyn Db, input: MyInput) -> u32 { - db.push_log(format!("result_deponds_on_y({:?})", input)); +fn result_depends_on_y(db: &dyn Db, input: MyInput) -> u32 { + db.push_log(format!("result_depends_on_y({:?})", input)); input.y(db) - 1 } @@ -53,33 +53,33 @@ impl HasLogger for Database { #[test] fn execute() { - // result_deponds_on_x = x + 1 - // result_deponds_on_y = y - 1 + // result_depends_on_x = x + 1 + // result_depends_on_y = y - 1 let mut db = Database::default(); let input = MyInput::new(&mut db, 22, 33); - assert_eq!(result_deponds_on_x(&db, input), 23); + assert_eq!(result_depends_on_x(&db, input), 23); db.assert_logs(expect![[r#" [ - "result_deponds_on_x(MyInput(Id { value: 1 }))", + "result_depends_on_x(MyInput(Id { value: 1 }))", ]"#]]); - assert_eq!(result_deponds_on_y(&db, input), 32); + assert_eq!(result_depends_on_y(&db, input), 32); db.assert_logs(expect![[r#" [ - "result_deponds_on_y(MyInput(Id { value: 1 }))", + "result_depends_on_y(MyInput(Id { value: 1 }))", ]"#]]); input.set_x(&mut db, 23); // input x changes, so result depends on x needs to be recomputed; - assert_eq!(result_deponds_on_x(&db, input), 24); + assert_eq!(result_depends_on_x(&db, input), 24); db.assert_logs(expect![[r#" [ - "result_deponds_on_x(MyInput(Id { value: 1 }))", + "result_depends_on_x(MyInput(Id { value: 1 }))", ]"#]]); // input y is the same, so result depends on y // does not need to be recomputed; - assert_eq!(result_deponds_on_y(&db, input), 32); + assert_eq!(result_depends_on_y(&db, input), 32); db.assert_logs(expect!["[]"]); } From e856f565b3dd4e04b1c6c73d337f53fc09cfee17 Mon Sep 17 00:00:00 2001 From: XFFXFF <1247714429@qq.com> Date: Mon, 22 Aug 2022 08:21:51 +0800 Subject: [PATCH 4/4] create inputs first in an lru test --- salsa-2022-tests/tests/lru.rs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/salsa-2022-tests/tests/lru.rs b/salsa-2022-tests/tests/lru.rs index 9c017437..bbb041b3 100644 --- a/salsa-2022-tests/tests/lru.rs +++ b/salsa-2022-tests/tests/lru.rs @@ -124,8 +124,9 @@ fn lru_can_be_changed_at_runtime() { let mut db = Database::default(); assert_eq!(load_n_potatoes(), 0); - for i in 0..128u32 { - let input = MyInput::new(&mut db, i); + let inputs: Vec<(u32, MyInput)> = (0..128).map(|i| (i, MyInput::new(&mut db, i))).collect(); + + for &(i, input) in inputs.iter() { let p = get_hot_potato(&db, input); assert_eq!(p.0, i) } @@ -136,8 +137,7 @@ fn lru_can_be_changed_at_runtime() { get_hot_potato::set_lru_capacity(&db, 64); assert_eq!(load_n_potatoes(), 32); - for i in 0..128u32 { - let input = MyInput::new(&mut db, i); + for &(i, input) in inputs.iter() { let p = get_hot_potato(&db, input); assert_eq!(p.0, i) } @@ -149,15 +149,14 @@ fn lru_can_be_changed_at_runtime() { // Special case: setting capacity to zero disables LRU get_hot_potato::set_lru_capacity(&db, 0); assert_eq!(load_n_potatoes(), 64); - for i in 0..128u32 { - let input = MyInput::new(&mut db, i); + for &(i, input) in inputs.iter() { let p = get_hot_potato(&db, input); assert_eq!(p.0, i) } // Create a new input to change the revision, and trigger the GC MyInput::new(&mut db, 0); - assert_eq!(load_n_potatoes(), 192); + assert_eq!(load_n_potatoes(), 128); drop(db); assert_eq!(load_n_potatoes(), 0);