refactor: switch to dashmap for Aspects and Registries

Signed-off-by: Schmarni <marnistromer@gmail.com>

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",

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"

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<T: Send + Sync + ?Sized>(Mutex<Option<FxHashMap<usize, Weak<T>>>>);
+pub struct Registry<T: Send + Sync + ?Sized>(MaybeLazy<DashMap<usize, Weak<T>>>);
 
 impl<T: Send + Sync + ?Sized> Registry<T> {
 	pub const fn new() -> Self {
-		Registry(const_mutex(None))
-	}
-	fn lock(&self) -> MappedMutexGuard<FxHashMap<usize, Weak<T>>> {
-		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<T>
 	where
@@ -24,30 +23,29 @@ impl<T: Send + Sync + ?Sized> Registry<T> {
 		t_arc
 	}
 	pub fn add_raw(&self, t: &Arc<T>) {
-		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<T>, new: &Registry<T>) -> (Vec<Arc<T>>, Vec<Arc<T>>) {
-		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<T: Send + Sync + ?Sized> Registry<T> {
 		(added, removed)
 	}
 	pub fn get_valid_contents(&self) -> Vec<Arc<T>> {
-		self.lock()
+		self.0
 			.iter()
-			.filter_map(|pair| pair.1.upgrade())
+			.filter_map(|pair| pair.value().upgrade())
 			.collect()
 	}
 	pub fn set(&self, other: &Registry<T>) {
-		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<Arc<T>> {
-		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<F: Fn(&Arc<T>) -> 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<T: Send + Sync + ?Sized> Clone for Registry<T> {
 	fn clone(&self) -> Self {
-		Self(Mutex::new(self.0.lock().clone()))
+		Self(self.0.clone())
 	}
 }
 impl<T: Send + Sync + ?Sized> Default for Registry<T> {
@@ -109,6 +103,30 @@ impl<T: Send + Sync + ?Sized> Default for Registry<T> {
 	}
 }
 
+#[derive(Debug)]
+enum MaybeLazy<T> {
+	Lazy(LazyLock<T>),
+	NonLazy(T),
+}
+impl<T: Clone> Clone for MaybeLazy<T> {
+	fn clone(&self) -> Self {
+		match self {
+			MaybeLazy::Lazy(lazy_lock) => Self::NonLazy(lazy_lock.deref().clone()),
+			MaybeLazy::NonLazy(v) => Self::NonLazy(v.clone()),
+		}
+	}
+}
+impl<T> Deref for MaybeLazy<T> {
+	type Target = T;
+
+	fn deref(&self) -> &Self::Target {
+		match self {
+			MaybeLazy::Lazy(lazy_lock) => lazy_lock,
+			MaybeLazy::NonLazy(v) => v,
+		}
+	}
+}
+
 pub struct OwnedRegistry<T: Send + Sync + ?Sized>(Mutex<Option<FxHashMap<usize, Arc<T>>>>);
 
 impl<T: Send + Sync + ?Sized> OwnedRegistry<T> {

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<FxHashMap<u64, Arc<dyn Aspect>>>);
+struct Aspects(DashMap<u64, Arc<dyn Aspect>>);
 impl Aspects {
 	fn add<A: AspectIdentifier>(&self, t: A) -> Arc<A> {
 		let aspect = Arc::new(t);
@@ -332,13 +330,13 @@ impl Aspects {
 		aspect
 	}
 	fn add_raw<A: AspectIdentifier>(&self, aspect: Arc<A>) {
-		self.0.lock().insert(A::ID, aspect);
+		self.0.insert(A::ID, aspect);
 	}
 	fn get<A: Aspect + AspectIdentifier>(&self) -> Result<Arc<A>> {
 		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::<A>()))
@@ -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()
 	}
 }