diff --git a/src/id.rs b/src/id.rs index 87ca9609a..2cd785dab 100644 --- a/src/id.rs +++ b/src/id.rs @@ -42,6 +42,7 @@ impl Id { /// salsa computations. #[doc(hidden)] #[track_caller] + #[inline] pub const unsafe fn from_u32(v: u32) -> Self { debug_assert!(v < Self::MAX_U32); Id { @@ -50,6 +51,7 @@ impl Id { } } + #[inline] pub const fn as_u32(self) -> u32 { self.value.get() - 1 } diff --git a/src/interned.rs b/src/interned.rs index 6d830f32e..f6aa1faba 100644 --- a/src/interned.rs +++ b/src/interned.rs @@ -1,6 +1,7 @@ #![allow(clippy::undocumented_unsafe_blocks)] // TODO(#697) document safety use std::any::TypeId; +use std::cell::Cell; use std::fmt; use std::hash::{BuildHasher, Hash, Hasher}; use std::marker::PhantomData; @@ -60,10 +61,14 @@ pub struct IngredientImpl { /// Maps from data to the existing interned id for that data. /// + /// This doesn't hold the fields themselves to save memory, instead it points to the slot ID. + /// /// Deadlock requirement: We access `value_map` while holding lock on `key_map`, but not vice versa. - key_map: FxDashMap, Id>, + key_map: FxDashMap, memo_table_types: Arc, + + _marker: PhantomData C>, } /// Struct storing the interned fields. @@ -135,6 +140,7 @@ where ingredient_index, key_map: Default::default(), memo_table_types: Arc::new(MemoTableTypes::default()), + _marker: PhantomData, } } @@ -193,14 +199,20 @@ where { let (zalsa, zalsa_local) = db.zalsas(); let current_revision = zalsa.current_revision(); + let table = zalsa.table(); // Optimization to only get read lock on the map if the data has already been interned. let data_hash = self.key_map.hasher().hash_one(&key); let shard = &self.key_map.shards()[self.key_map.determine_shard(data_hash as _)]; - let eq = |(data, _): &_| { + let found_value = Cell::new(None); + let eq = |(id, _): &_| { + let data = table.get::>(*id); + found_value.set(Some(data)); // SAFETY: it's safe to go from Data<'static> to Data<'db> // shrink lifetime here to use a single lifetime in Lookup::eq(&StructKey<'db>, &C::Data<'db>) - let data: &C::Fields<'db> = unsafe { std::mem::transmute(data) }; + let data = unsafe { + std::mem::transmute::<&C::Fields<'static>, &C::Fields<'db>>(&data.fields) + }; HashEqLike::eq(data, &key) }; @@ -208,9 +220,11 @@ where let lock = shard.read(); if let Some(bucket) = lock.find(data_hash, eq) { // SAFETY: Read lock on map is held during this block - let id = unsafe { *bucket.as_ref().1.get() }; + let id = unsafe { bucket.as_ref().0 }; - let value = zalsa.table().get::>(id); + let value = found_value + .get() + .expect("found the interned, so `found_value` should be set"); // Sync the value's revision. if value.last_interned_at.load() < current_revision { @@ -243,12 +257,16 @@ where } let mut lock = shard.write(); - match lock.find_or_find_insert_slot(data_hash, eq, |(element, _)| { - self.key_map.hasher().hash_one(element) + match lock.find_or_find_insert_slot(data_hash, eq, |(id, _)| { + // This closure is only called if the table is resized. So while it's expensive to lookup all values, + // it will only happen rarely. + self.key_map + .hasher() + .hash_one(&table.get::>(*id).fields) }) { // Data has been interned by a racing call, use that ID instead Ok(slot) => { - let id = unsafe { *slot.as_ref().1.get() }; + let id = unsafe { slot.as_ref().0 }; let value = zalsa.table().get::>(id); // Sync the value's revision. @@ -302,8 +320,7 @@ where let value = zalsa.table().get::>(id); - let slot_value = (value.fields.clone(), SharedValue::new(id)); - unsafe { lock.insert_in_slot(data_hash, slot, slot_value) }; + unsafe { lock.insert_in_slot(data_hash, slot, (id, SharedValue::new(()))) }; debug_assert_eq!( data_hash, diff --git a/src/table.rs b/src/table.rs index 113b3146e..89afc453f 100644 --- a/src/table.rs +++ b/src/table.rs @@ -164,6 +164,7 @@ impl PageIndex { struct SlotIndex(usize); impl SlotIndex { + #[inline] fn new(idx: usize) -> Self { debug_assert!(idx < PAGE_LEN); Self(idx) @@ -218,6 +219,7 @@ impl Table { /// # Panics /// /// If `page` is out of bounds or the type `T` is incorrect. + #[inline] pub(crate) fn page(&self, page: PageIndex) -> PageView { self.pages[page.0].assert_type::() } @@ -317,6 +319,7 @@ impl<'p, T: Slot> PageView<'p, T> { unsafe { slice::from_raw_parts(self.0.data.cast::>().as_ptr(), len) } } + #[inline] fn data(&self) -> &'p [T] { let len = self.0.allocated.load(Ordering::Acquire); // SAFETY: `len` is the initialized length of the page @@ -385,6 +388,7 @@ impl Page { } } + #[inline] fn assert_type(&self) -> PageView { assert_eq!( self.slot_type_id, @@ -420,6 +424,7 @@ fn make_id(page: PageIndex, slot: SlotIndex) -> Id { unsafe { Id::from_u32((page << PAGE_LEN_BITS) | slot) } } +#[inline] fn split_id(id: Id) -> (PageIndex, SlotIndex) { let id = id.as_u32() as usize; let slot = id & PAGE_LEN_MASK;