memcache: generalize indexing

This should also improve performance a little
because entries are now behind an Arc, so the
lock has to be held for shorter time periods.

src/data/account/mem.rs

@@ -1,6 +1,6 @@
 use crate::core::*;
 
-use crate::data::memcache::{GetId, MemCache};
+use crate::data::memcache::{Indexable, MemCache};
 use crate::model::Account;
 
 pub struct MemAccountDataSource {
@@ -15,7 +15,7 @@ impl MemAccountDataSource {
     }
 
     pub async fn by_id(&self, id: Id) -> Option<Account> {
-        self.cache.get(id).await
+        self.cache.get(&id).await
     }
 
     pub async fn store(&self, account: Account) {
@@ -23,8 +23,8 @@ impl MemAccountDataSource {
     }
 }
 
-impl GetId<Id> for Account {
+impl Indexable<Id> for Account {
-    fn get_id(&self) -> Id {
+    fn get_id(&self) -> &Id {
-        self.id
+        &self.id
     }
 }

src/data/memcache.rs

@@ -1,24 +1,26 @@
+use std::hash::{Hash, Hasher};
 use std::marker::PhantomData;
+use std::sync::Arc;
 use tokio::sync::RwLock;
 use uuid::Uuid;
 
 use crate::core::*;
 
 /// Shared in-memory LRU with multiple cache lines.
-pub struct MemCache<I: IndexableId, T: GetId<I> + Clone> {
+/// Basically a [`HashMap`] that is [`Sync`] and randomly forgets old entries.
-    lines: Vec<RwLock<Option<T>>>,
+pub struct MemCache<I: HashableId, T: Indexable<I>> {
+    lines: Vec<RwLock<Option<Arc<T>>>>,
     _phantom: PhantomData<I>,
 }
 
-pub trait GetId<I: IndexableId> {
+pub trait HashableId: Hash + PartialEq + Send {}
-    fn get_id(&self) -> I;
+impl<T> HashableId for T where T: Hash + PartialEq + Send {}
+
+pub trait Indexable<I: HashableId>: Clone + Send {
+    fn get_id(&self) -> &I;
 }
 
-pub trait IndexableId: Eq {
+impl<I: HashableId, T: Indexable<I>> MemCache<I, T> {
-    fn compute_index(&self, modulus: usize) -> usize;
-}
-
-impl<I: IndexableId, T: GetId<I> + Clone> MemCache<I, T> {
     pub fn new() -> MemCache<I, T> {
         MemCache::with_capacity(256)
     }
@@ -35,43 +37,68 @@ impl<I: IndexableId, T: GetId<I> + Clone> MemCache<I, T> {
         }
     }
 
-    pub async fn get(&self, id: I) -> Option<T> {
+    pub async fn get(&self, id: &I) -> Option<T> {
-        let index = id.compute_index(self.lines.len());
+        let index = self.compute_index(id);
         let guard = self.lines[index].read().await;
-        let entry = guard.as_ref()?;
+        let entry = Arc::clone(guard.as_ref()?);
+        drop(guard);
+
         if entry.get_id() == id {
-            Some(entry.clone())
+            Some(entry.as_ref().clone())
         } else {
             None
         }
     }
 
     pub async fn put(&self, entry: T) {
-        let index = entry.get_id().compute_index(self.lines.len());
+        let index = self.compute_index(entry.get_id());
+        let entry = Arc::new(entry);
         let mut guard = self.lines[index].write().await;
-        *guard = Some(entry);
+        let old_entry = guard.replace(entry);
+        // explicitly drop the lock before the old entry
+        // so we never deallocate while holding the lock
+        // (unless rustc is trying to be smarter than us)
+        drop(guard);
+        drop(old_entry);
     }
 
-    pub async fn del(&self, id: I) {
+    pub async fn del(&self, id: &I) {
-        let index = id.compute_index(self.lines.len());
+        let index = self.compute_index(id);
         let mut guard = self.lines[index].write().await;
         if let Some(entry) = guard.as_ref() {
             if entry.get_id() == id {
-                *guard = None;
+                let old_entry = guard.take();
+                // same thing as with put(), don't deallocate while holding the lock
+                drop(guard);
+                drop(old_entry);
             }
         }
     }
-}
 
-impl IndexableId for Id {
+    fn compute_index(&self, id: &I) -> usize {
-    fn compute_index(&self, modulus: usize) -> usize {
+        let mut hasher = Djb2::new();
-        *self as usize % modulus
+        id.hash(&mut hasher);
+        hasher.finish() as usize % self.lines.len()
     }
 }
 
-impl IndexableId for Uuid {
+struct Djb2 {
-    fn compute_index(&self, modulus: usize) -> usize {
+    hash: u64,
-        self.as_u128() as usize % modulus
+}
+
+impl Djb2 {
+    pub fn new() -> Djb2 {
+        Djb2 { hash: 5381 }
+    }
+}
+
+impl Hasher for Djb2 {
+    fn finish(&self) -> u64 {
+        self.hash
+    }
+
+    fn write(&mut self, bytes: &[u8]) {
+        self.hash = bytes.iter().fold(self.hash, |h, b| h * 33 ^ (*b as u64));
     }
 }
 
@@ -84,34 +111,24 @@ mod tests {
         id: Id,
     }
 
-    impl GetId<Id> for TestEntry {
+    impl Indexable<Id> for TestEntry {
-        fn get_id(&self) -> Id {
+        fn get_id(&self) -> &Id {
-            self.id
+            &self.id
         }
     }
 
     #[actix_web::test]
     async fn store_stuff() {
         let cache = MemCache::with_capacity(256);
-        for id in 0..256 {
+        for id in 0..1024 {
             let entry = TestEntry { id };
             cache.put(entry.clone()).await;
-            assert_eq!(cache.get(id).await, Some(entry));
+            assert_eq!(cache.get(&id).await, Some(entry));
-        }
-        for id in 256..512 {
-            let entry = TestEntry { id };
-            cache.put(entry.clone()).await;
-            assert_eq!(cache.get(id - 256).await, None);
-            assert_eq!(cache.get(id).await, Some(entry));
         }
 
-        for id in 0..256 {
+        for id in 0..1024 {
-            cache.del(id).await;
+            cache.del(&id).await;
-            assert!(cache.get(id + 256).await.is_some());
+            assert_eq!(cache.get(&id).await, None);
-        }
-        for id in 256..512 {
-            cache.del(id).await;
-            assert_eq!(cache.get(id).await, None);
         }
     }
 }

src/data/note/mem.rs

@@ -1,6 +1,6 @@
 use crate::core::*;
 
-use crate::data::memcache::{GetId, MemCache};
+use crate::data::memcache::{Indexable, MemCache};
 use crate::model::Note;
 
 pub struct MemNoteDataSource {
@@ -15,7 +15,7 @@ impl MemNoteDataSource {
     }
 
     pub async fn by_id(&self, id: Id) -> Option<Note> {
-        self.cache.get(id).await
+        self.cache.get(&id).await
     }
 
     pub async fn store(&self, note: Note) {
@@ -23,12 +23,12 @@ impl MemNoteDataSource {
     }
 
     pub async fn delete(&self, id: Id) {
-        self.cache.del(id).await;
+        self.cache.del(&id).await;
     }
 }
 
-impl GetId<Id> for Note {
+impl Indexable<Id> for Note {
-    fn get_id(&self) -> Id {
+    fn get_id(&self) -> &Id {
-        self.id
+        &self.id
     }
 }

src/data/object/mem.rs

@@ -1,6 +1,6 @@
 use uuid::Uuid;
 
-use crate::data::memcache::{GetId, MemCache};
+use crate::data::memcache::{Indexable, MemCache};
 use crate::model::Object;
 
 pub struct MemObjectDataSource {
@@ -15,7 +15,7 @@ impl MemObjectDataSource {
     }
 
     pub async fn by_id(&self, id: Uuid) -> Option<Object> {
-        self.cache.get(id).await
+        self.cache.get(&id).await
     }
 
     pub async fn store(&self, obj: Object) {
@@ -23,8 +23,8 @@ impl MemObjectDataSource {
     }
 }
 
-impl GetId<Uuid> for Object {
+impl Indexable<Uuid> for Object {
-    fn get_id(&self) -> Uuid {
+    fn get_id(&self) -> &Uuid {
-        self.id
+        &self.id
     }
 }

src/data/user/mem.rs

@@ -1,6 +1,6 @@
 use crate::core::*;
 
-use crate::data::memcache::{GetId, MemCache};
+use crate::data::memcache::{Indexable, MemCache};
 use crate::model::User;
 
 pub struct MemUserDataSource {
@@ -15,7 +15,7 @@ impl MemUserDataSource {
     }
 
     pub async fn by_id(&self, id: Id) -> Option<User> {
-        self.cache.get(id).await
+        self.cache.get(&id).await
     }
 
     pub async fn store(&self, user: User) {
@@ -23,8 +23,8 @@ impl MemUserDataSource {
     }
 }
 
-impl GetId<Id> for User {
+impl Indexable<Id> for User {
-    fn get_id(&self) -> Id {
+    fn get_id(&self) -> &Id {
-        self.id
+        &self.id
     }
 }