Skip to content

Commit

Permalink
Auto merge of #16643 - Veykril:salsa-opt, r=Veykril
Browse files Browse the repository at this point in the history
internal: Optimize salsa memory usage

Reduces memory on self by ~20mb for me, there is a few more mb to save here if we made LRU caching opt-in, as currently every entry in a memoized query will store an `AtomicUsize` for the LRU.
  • Loading branch information
bors committed Feb 23, 2024
2 parents deddea0 + cc4d0e1 commit cbc579e
Show file tree
Hide file tree
Showing 18 changed files with 174 additions and 163 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion crates/ide-db/src/apply_change.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ impl RootDatabase {
pub fn request_cancellation(&mut self) {
let _p =
tracing::span!(tracing::Level::INFO, "RootDatabase::request_cancellation").entered();
self.salsa_runtime_mut().synthetic_write(Durability::LOW);
self.synthetic_write(Durability::LOW);
}

pub fn apply_change(&mut self, change: Change) {
Expand Down
1 change: 1 addition & 0 deletions crates/salsa/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ rustc-hash = "1.0"
smallvec = "1.0.0"
oorandom = "11"
triomphe = "0.1.11"
itertools.workspace = true

salsa-macros = { version = "0.0.0", path = "salsa-macros" }

Expand Down
4 changes: 2 additions & 2 deletions crates/salsa/salsa-macros/src/database_storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,8 +154,8 @@ pub(crate) fn database(args: TokenStream, input: TokenStream) -> TokenStream {
self.#db_storage_field.salsa_runtime()
}

fn ops_salsa_runtime_mut(&mut self) -> &mut salsa::Runtime {
self.#db_storage_field.salsa_runtime_mut()
fn synthetic_write(&mut self, durability: salsa::Durability) {
self.#db_storage_field.salsa_runtime_mut().synthetic_write(durability)
}

fn fmt_index(
Expand Down
4 changes: 2 additions & 2 deletions crates/salsa/salsa-macros/src/query_group.rs
Original file line number Diff line number Diff line change
Expand Up @@ -526,7 +526,7 @@ pub(crate) fn query_group(args: TokenStream, input: TokenStream) -> TokenStream
fmt_ops.extend(quote! {
#query_index => {
salsa::plumbing::QueryStorageOps::fmt_index(
&*self.#fn_name, db, input, fmt,
&*self.#fn_name, db, input.key_index(), fmt,
)
}
});
Expand All @@ -537,7 +537,7 @@ pub(crate) fn query_group(args: TokenStream, input: TokenStream) -> TokenStream
maybe_changed_ops.extend(quote! {
#query_index => {
salsa::plumbing::QueryStorageOps::maybe_changed_after(
&*self.#fn_name, db, input, revision
&*self.#fn_name, db, input.key_index(), revision
)
}
});
Expand Down
25 changes: 12 additions & 13 deletions crates/salsa/src/derived.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,13 +102,13 @@ where

let mut write = self.slot_map.write();
let entry = write.entry(key.clone());
let key_index = u32::try_from(entry.index()).unwrap();
let key_index = entry.index() as u32;
let database_key_index = DatabaseKeyIndex {
group_index: self.group_index,
query_index: Q::QUERY_INDEX,
key_index,
};
entry.or_insert_with(|| Arc::new(Slot::new(key.clone(), database_key_index))).clone()
entry.or_insert_with(|| Arc::new(Slot::new(database_key_index))).clone()
}
}

Expand All @@ -131,34 +131,33 @@ where
fn fmt_index(
&self,
_db: &<Q as QueryDb<'_>>::DynDb,
index: DatabaseKeyIndex,
index: u32,
fmt: &mut std::fmt::Formatter<'_>,
) -> std::fmt::Result {
assert_eq!(index.group_index, self.group_index);
assert_eq!(index.query_index, Q::QUERY_INDEX);
let slot_map = self.slot_map.read();
let key = slot_map.get_index(index.key_index as usize).unwrap().0;
let key = slot_map.get_index(index as usize).unwrap().0;
write!(fmt, "{}({:?})", Q::QUERY_NAME, key)
}

fn maybe_changed_after(
&self,
db: &<Q as QueryDb<'_>>::DynDb,
input: DatabaseKeyIndex,
index: u32,
revision: Revision,
) -> bool {
assert_eq!(input.group_index, self.group_index);
assert_eq!(input.query_index, Q::QUERY_INDEX);
debug_assert!(revision < db.salsa_runtime().current_revision());
let slot = self.slot_map.read().get_index(input.key_index as usize).unwrap().1.clone();
slot.maybe_changed_after(db, revision)
let read = &self.slot_map.read();
let Some((key, slot)) = read.get_index(index as usize) else {
return false;
};
slot.maybe_changed_after(db, revision, key)
}

fn fetch(&self, db: &<Q as QueryDb<'_>>::DynDb, key: &Q::Key) -> Q::Value {
db.unwind_if_cancelled();

let slot = self.slot(key);
let StampedValue { value, durability, changed_at } = slot.read(db);
let StampedValue { value, durability, changed_at } = slot.read(db, key);

if let Some(evicted) = self.lru_list.record_use(&slot) {
evicted.evict();
Expand All @@ -182,7 +181,7 @@ where
C: std::iter::FromIterator<TableEntry<Q::Key, Q::Value>>,
{
let slot_map = self.slot_map.read();
slot_map.values().filter_map(|slot| slot.as_table_entry()).collect()
slot_map.iter().filter_map(|(key, slot)| slot.as_table_entry(key)).collect()
}
}

Expand Down
Loading

0 comments on commit cbc579e

Please sign in to comment.