diff --git a/benches/compare.rs b/benches/compare.rs index 2a2e5394e..b9444795e 100644 --- a/benches/compare.rs +++ b/benches/compare.rs @@ -21,11 +21,25 @@ pub struct InternedInput<'db> { pub text: String, } +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, salsa::Supertype)] +enum EnumInput<'db> { + InternedInput(InternedInput<'db>), + Input(Input), +} + #[salsa::tracked] pub fn interned_length<'db>(db: &'db dyn salsa::Database, input: InternedInput<'db>) -> usize { input.text(db).len() } +#[salsa::tracked] +pub fn either_length<'db>(db: &'db dyn salsa::Database, input: EnumInput<'db>) -> usize { + match input { + EnumInput::InternedInput(input) => interned_length(db, input), + EnumInput::Input(input) => length(db, input), + } +} + fn mutating_inputs(c: &mut Criterion) { let mut group: codspeed_criterion_compat::BenchmarkGroup< codspeed_criterion_compat::measurement::WallTime, @@ -146,6 +160,77 @@ fn inputs(c: &mut Criterion) { ) }); + group.bench_function(BenchmarkId::new("new", "EnumInput"), |b| { + b.iter_batched_ref( + || { + let db = salsa::DatabaseImpl::default(); + + // Prepopulate ingredients. + let input = EnumInput::Input(Input::new( + black_box(&db), + black_box("hello, world!".to_owned()), + )); + let interned_input = EnumInput::InternedInput(InternedInput::new( + black_box(&db), + black_box("hello, world!".to_owned()), + )); + let len = either_length(black_box(&db), black_box(input)); + assert_eq!(black_box(len), 13); + let len = either_length(black_box(&db), black_box(interned_input)); + assert_eq!(black_box(len), 13); + + db + }, + |db| { + let input = EnumInput::Input(Input::new( + black_box(db), + black_box("hello, world!".to_owned()), + )); + let interned_input = EnumInput::InternedInput(InternedInput::new( + black_box(db), + black_box("hello, world!".to_owned()), + )); + let len = either_length(black_box(db), black_box(input)); + assert_eq!(black_box(len), 13); + let len = either_length(black_box(db), black_box(interned_input)); + assert_eq!(black_box(len), 13); + }, + BatchSize::SmallInput, + ) + }); + + group.bench_function(BenchmarkId::new("amortized", "EnumInput"), |b| { + b.iter_batched_ref( + || { + let db = salsa::DatabaseImpl::default(); + + let input = EnumInput::Input(Input::new( + black_box(&db), + black_box("hello, world!".to_owned()), + )); + let interned_input = EnumInput::InternedInput(InternedInput::new( + black_box(&db), + black_box("hello, world!".to_owned()), + )); + // we can't pass this along otherwise, and the lifetime is generally informational + let interned_input: EnumInput<'static> = unsafe { transmute(interned_input) }; + let len = either_length(black_box(&db), black_box(input)); + assert_eq!(black_box(len), 13); + let len = either_length(black_box(&db), black_box(interned_input)); + assert_eq!(black_box(len), 13); + + (db, input, interned_input) + }, + |&mut (ref db, input, interned_input)| { + let len = either_length(black_box(db), black_box(input)); + assert_eq!(black_box(len), 13); + let len = either_length(black_box(db), black_box(interned_input)); + assert_eq!(black_box(len), 13); + }, + BatchSize::SmallInput, + ) + }); + group.finish(); } diff --git a/src/function.rs b/src/function.rs index cfaa17298..33bf8536f 100644 --- a/src/function.rs +++ b/src/function.rs @@ -96,8 +96,9 @@ pub struct IngredientImpl { /// The index for the memo/sync tables /// - /// This may be a `MemoIngredientSingletonIndex` or a `MemoIngredientIndex`, depending on - /// whether the tracked function's struct is a plain salsa struct or an enum `#[derive(Supertype)]`. + /// This may be a [`crate::memo_ingredient_indices::MemoIngredientSingletonIndex`] or a + /// [`crate::memo_ingredient_indices::MemoIngredientIndices`], depending on whether the + /// tracked function's struct is a plain salsa struct or an enum `#[derive(Supertype)]`. memo_ingredient_indices: as SalsaStructInDb>::MemoIngredientMap, /// Used to find memos to throw out when we have too many memoized values. diff --git a/src/zalsa.rs b/src/zalsa.rs index 744e5c912..18a26c9a7 100644 --- a/src/zalsa.rs +++ b/src/zalsa.rs @@ -8,7 +8,6 @@ use std::panic::RefUnwindSafe; use std::thread::ThreadId; use crate::cycle::CycleRecoveryStrategy; -use crate::hash::FxDashMap; use crate::ingredient::{Ingredient, Jar}; use crate::nonce::{Nonce, NonceGenerator}; use crate::runtime::{Runtime, WaitResult}; @@ -140,7 +139,9 @@ pub struct Zalsa { jar_map: Mutex>, /// A map from the `IngredientIndex` to the `TypeId` of its ID struct. - ingredient_to_id_struct_type_id_map: FxDashMap, + /// + /// Notably this is not the reverse mapping of `jar_map`. + ingredient_to_id_struct_type_id_map: RwLock>, /// Vector of ingredients. /// @@ -225,6 +226,7 @@ impl Zalsa { let ingredient_index = self.ingredient_index(id); *self .ingredient_to_id_struct_type_id_map + .read() .get(&ingredient_index) .expect("should have the ingredient index available") } @@ -236,6 +238,7 @@ impl Zalsa { if let Some(index) = jar_map.get(&jar_type_id) { return *index; }; + // Drop the map as `J::create_dependencies` may recurse into this function taking the lock again. drop(jar_map); let dependencies = J::create_dependencies(self); @@ -269,6 +272,7 @@ impl Zalsa { drop(jar_map); self.ingredient_to_id_struct_type_id_map + .write() .insert(index, J::id_struct_type_id()); index