From 8fc017a6fc0919f1e91e6b4c3b1f11166df623a1 Mon Sep 17 00:00:00 2001 From: Schmarni Date: Wed, 19 Mar 2025 16:51:59 +0100 Subject: [PATCH] refactor: switch to dashmap for Aspects and Registries Signed-off-by: Schmarni --- Cargo.lock | 15 ++++++++ Cargo.toml | 1 + src/core/registry.rs | 90 ++++++++++++++++++++++++++------------------ src/nodes/mod.rs | 17 ++++----- 4 files changed, 78 insertions(+), 45 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d30cf1d..131e907 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -755,6 +755,20 @@ version = "1.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "96a6ac251f4a2aca6b3f91340350eab87ae57c3f127ffeb585e92bd336717991" +[[package]] +name = "dashmap" +version = "6.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5041cc499144891f3790297212f32a74fb938e5136a14943f338ef9e0ae276cf" +dependencies = [ + "cfg-if", + "crossbeam-utils", + "hashbrown 0.14.5", + "lock_api", + "once_cell", + "parking_lot_core 0.9.10", +] + [[package]] name = "directories" version = "5.0.1" @@ -2653,6 +2667,7 @@ dependencies = [ "clap", "color-eyre", "console-subscriber", + "dashmap", "directories", "glam", "global_counter", diff --git a/Cargo.toml b/Cargo.toml index 15b55cc..e494c63 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -92,6 +92,7 @@ xkbcommon-rs = "0.1.0" # wayland wayland-backend = { version = "0.3.7", optional = true, default-features = false } wayland-scanner = { version = "0.31.4", optional = true } +dashmap = "6.1.0" [dependencies.smithay] git = "https://github.com/smithay/smithay.git" diff --git a/src/core/registry.rs b/src/core/registry.rs index 5d9d85b..d1e5431 100644 --- a/src/core/registry.rs +++ b/src/core/registry.rs @@ -1,19 +1,18 @@ #![allow(dead_code)] +use dashmap::DashMap; use parking_lot::{MappedMutexGuard, Mutex, MutexGuard, const_mutex}; use rustc_hash::FxHashMap; +use std::ops::Deref; use std::ptr; -use std::sync::{Arc, Weak}; +use std::sync::{Arc, LazyLock, Weak}; #[derive(Debug)] -pub struct Registry(Mutex>>>); +pub struct Registry(MaybeLazy>>); impl Registry { pub const fn new() -> Self { - Registry(const_mutex(None)) - } - fn lock(&self) -> MappedMutexGuard>> { - MutexGuard::map(self.0.lock(), |r| r.get_or_insert_with(FxHashMap::default)) + Registry(MaybeLazy::Lazy(LazyLock::new(DashMap::default))) } pub fn add(&self, t: T) -> Arc where @@ -24,30 +23,29 @@ impl Registry { t_arc } pub fn add_raw(&self, t: &Arc) { - self.lock() + self.0 .insert(Arc::as_ptr(t) as *const () as usize, Arc::downgrade(t)); } pub fn contains(&self, t: &T) -> bool { - self.lock() + self.0 .contains_key(&(ptr::addr_of!(*t) as *const () as usize)) } pub fn get_changes(old: &Registry, new: &Registry) -> (Vec>, Vec>) { - let old = old.lock(); - let new = new.lock(); - let mut added = Vec::new(); let mut removed = Vec::new(); - for (id, entry) in new.iter() { + for pair in new.0.iter() { + let (id, entry) = pair.pair(); if let Some(entry) = entry.upgrade() { - if !old.contains_key(id) { + if !old.0.contains_key(id) { added.push(entry); } } } - for (id, entry) in old.iter() { + for pair in old.0.iter() { + let (id, entry) = pair.pair(); if let Some(entry) = entry.upgrade() { - if !new.contains_key(id) { + if !new.0.contains_key(id) { removed.push(entry); } } @@ -55,52 +53,48 @@ impl Registry { (added, removed) } pub fn get_valid_contents(&self) -> Vec> { - self.lock() + self.0 .iter() - .filter_map(|pair| pair.1.upgrade()) + .filter_map(|pair| pair.value().upgrade()) .collect() } pub fn set(&self, other: &Registry) { - self.lock().clone_from(&other.lock()); + self.clear(); + for (key, value) in other.0.deref().clone().into_iter() { + self.0.insert(key, value); + } } pub fn take_valid_contents(&self) -> Vec> { - self.0 - .lock() - .take() - .unwrap_or_default() - .into_iter() - .filter_map(|pair| pair.1.upgrade()) - .collect() + let contents = self.get_valid_contents(); + self.0.clear(); + contents } pub fn retain) -> bool>(&self, f: F) { - self.lock().retain(|_, v| { + self.0.retain(|_, v| { let Some(v) = v.upgrade() else { + // why would we want to retain things we can't upgrade? return true; }; (f)(&v) }) } pub fn remove(&self, t: &T) { - self.lock() - .remove(&(ptr::addr_of!(*t) as *const () as usize)); + self.0.remove(&(ptr::addr_of!(*t) as *const () as usize)); } pub fn clear(&self) { - self.lock().clear(); + self.0.clear(); } pub fn is_empty(&self) -> bool { - let registry = self.0.lock(); - let Some(registry) = &*registry else { - return true; - }; - if registry.is_empty() { + if self.0.is_empty() { return true; } - registry.values().all(|v| v.strong_count() == 0) + self.0.iter().all(|v| v.value().strong_count() == 0) } } + impl Clone for Registry { fn clone(&self) -> Self { - Self(Mutex::new(self.0.lock().clone())) + Self(self.0.clone()) } } impl Default for Registry { @@ -109,6 +103,30 @@ impl Default for Registry { } } +#[derive(Debug)] +enum MaybeLazy { + Lazy(LazyLock), + NonLazy(T), +} +impl Clone for MaybeLazy { + fn clone(&self) -> Self { + match self { + MaybeLazy::Lazy(lazy_lock) => Self::NonLazy(lazy_lock.deref().clone()), + MaybeLazy::NonLazy(v) => Self::NonLazy(v.clone()), + } + } +} +impl Deref for MaybeLazy { + type Target = T; + + fn deref(&self) -> &Self::Target { + match self { + MaybeLazy::Lazy(lazy_lock) => lazy_lock, + MaybeLazy::NonLazy(v) => v, + } + } +} + pub struct OwnedRegistry(Mutex>>>); impl OwnedRegistry { diff --git a/src/nodes/mod.rs b/src/nodes/mod.rs index fe1359e..3296613 100644 --- a/src/nodes/mod.rs +++ b/src/nodes/mod.rs @@ -12,8 +12,7 @@ use crate::core::client::Client; use crate::core::error::{Result, ServerError}; use crate::core::registry::Registry; use crate::core::scenegraph::MethodResponseSender; -use parking_lot::Mutex; -use rustc_hash::FxHashMap; +use dashmap::DashMap; use serde::{Serialize, de::DeserializeOwned}; use spatial::Spatial; use stardust_xr::messenger::MessageSenderHandle; @@ -190,7 +189,6 @@ impl Node { let aspect = self .aspects .0 - .lock() .get(&aspect_id) .ok_or(ScenegraphError::AspectNotFound)? .clone(); @@ -229,7 +227,7 @@ impl Node { response, ) } else { - let Some(aspect) = self.aspects.0.lock().get(&aspect_id).cloned() else { + let Some(aspect) = self.aspects.0.get(&aspect_id).map(|v| v.clone()) else { response.send(Err(ScenegraphError::AspectNotFound)); return; }; @@ -324,7 +322,7 @@ pub trait Aspect: Any + Send + Sync + 'static { } #[derive(Default)] -struct Aspects(Mutex>>); +struct Aspects(DashMap>); impl Aspects { fn add(&self, t: A) -> Arc { let aspect = Arc::new(t); @@ -332,13 +330,13 @@ impl Aspects { aspect } fn add_raw(&self, aspect: Arc) { - self.0.lock().insert(A::ID, aspect); + self.0.insert(A::ID, aspect); } fn get(&self) -> Result> { self.0 - .lock() .get(&A::ID) - .cloned() + // .cloned doesn't work for some reason + .map(|v| v.clone()) .map(|a| a.as_any()) .and_then(|a| Arc::downcast(a).ok()) .ok_or(ServerError::NoAspect(TypeId::of::())) @@ -346,6 +344,7 @@ impl Aspects { } impl Drop for Aspects { fn drop(&mut self) { - self.0.lock().clear() + // why would this be needed? do drop impls not run otherwise? + self.0.clear() } }