Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 85 additions & 0 deletions benches/compare.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,25 @@ pub struct InternedInput<'db> {
pub text: String,
}

#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, salsa::Supertype)]
enum EnumInput<'db> {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leaving this as a note for myself: I'll rename this to SupertypeInput.

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,
Expand Down Expand Up @@ -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();
}

Expand Down
5 changes: 3 additions & 2 deletions src/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,9 @@ pub struct IngredientImpl<C: Configuration> {

/// 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: <C::SalsaStruct<'static> as SalsaStructInDb>::MemoIngredientMap,

/// Used to find memos to throw out when we have too many memoized values.
Expand Down
8 changes: 6 additions & 2 deletions src/zalsa.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -140,7 +139,9 @@ pub struct Zalsa {
jar_map: Mutex<FxHashMap<TypeId, IngredientIndex>>,

/// A map from the `IngredientIndex` to the `TypeId` of its ID struct.
ingredient_to_id_struct_type_id_map: FxDashMap<IngredientIndex, TypeId>,
///
/// Notably this is not the reverse mapping of `jar_map`.
ingredient_to_id_struct_type_id_map: RwLock<FxHashMap<IngredientIndex, TypeId>>,

/// Vector of ingredients.
///
Expand Down Expand Up @@ -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")
}
Expand All @@ -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);

Expand Down Expand Up @@ -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
Expand Down
Loading