Merge pull request #1371 from hannobraun/store

Make some cleanups in store code

crates/fj-kernel/src/storage/blocks.rs

@@ -19,7 +19,7 @@ impl<T> Blocks<T> {
         self.insert(index, object)
     }
 
-    pub fn reserve(&mut self) -> ((usize, usize), *mut Option<T>) {
+    pub fn reserve(&mut self) -> (Index, *const Option<T>) {
         let mut current_block = match self.inner.pop() {
             Some(block) => block,
             None => Block::new(self.block_size),
@@ -28,8 +28,14 @@ impl<T> Blocks<T> {
         let ret = loop {
             match current_block.reserve() {
                 Ok((object_index, ptr)) => {
-                    let block_index = self.inner.len();
-                    break ((block_index, object_index), ptr);
+                    let block_index = BlockIndex(self.inner.len());
+                    break (
+                        Index {
+                            block_index,
+                            object_index,
+                        },
+                        ptr,
+                    );
                 }
                 Err(()) => {
                     // Block is full. Need to create a new one and retry.
@@ -44,13 +50,9 @@ impl<T> Blocks<T> {
         ret
     }
 
-    pub fn insert(
-        &mut self,
-        (block_index, object_index): (usize, usize),
-        object: T,
-    ) -> *const Option<T> {
-        let block = &mut self.inner[block_index];
-        block.insert(object_index, object)
+    pub fn insert(&mut self, index: Index, object: T) -> *const Option<T> {
+        let block = &mut self.inner[index.block_index.0];
+        block.insert(index.object_index, object)
     }
 
     pub fn get(&self, index: usize) -> Option<&Block<T>> {
@@ -66,7 +68,7 @@ impl<T> Blocks<T> {
 #[derive(Debug)]
 pub struct Block<T> {
     objects: Box<[Option<T>]>,
-    next: usize,
+    next: ObjectIndex,
 }
 
 impl<T> Block<T> {
@@ -76,32 +78,39 @@ impl<T> Block<T> {
             .collect::<Vec<Option<T>>>();
         let objects = vec.into_boxed_slice();
 
-        Self { objects, next: 0 }
+        Self {
+            objects,
+            next: ObjectIndex(0),
+        }
     }
 
-    pub fn reserve(&mut self) -> Result<(usize, *mut Option<T>), ()> {
-        if self.next >= self.objects.len() {
+    pub fn reserve(&mut self) -> Result<(ObjectIndex, *const Option<T>), ()> {
+        if self.next.0 >= self.objects.len() {
             return Err(());
         }
 
         let index = self.next;
-        let ptr = &mut self.objects[self.next];
-        self.next += 1;
+        let ptr = &mut self.objects[self.next.0];
+        self.next.0 += 1;
 
         Ok((index, ptr))
     }
 
-    pub fn insert(&mut self, index: usize, object: T) -> *const Option<T> {
-        self.objects[index] = Some(object);
-        &self.objects[index]
+    pub fn insert(
+        &mut self,
+        index: ObjectIndex,
+        object: T,
+    ) -> *const Option<T> {
+        self.objects[index.0] = Some(object);
+        &self.objects[index.0]
     }
 
     pub fn get(&self, index: usize) -> &Option<T> {
         &self.objects[index]
     }
 
     pub fn len(&self) -> usize {
-        self.next
+        self.next.0
     }
 
     #[cfg(test)]
@@ -120,6 +129,18 @@ impl<T> Block<T> {
     }
 }
 
+#[derive(Clone, Copy, Debug)]
+pub struct Index {
+    block_index: BlockIndex,
+    object_index: ObjectIndex,
+}
+
+#[derive(Clone, Copy, Debug)]
+pub struct BlockIndex(usize);
+
+#[derive(Clone, Copy, Debug)]
+pub struct ObjectIndex(usize);
+
 #[cfg(test)]
 mod tests {
     use super::Blocks;

crates/fj-kernel/src/storage/handle.rs

@@ -72,11 +72,11 @@ impl<T> Deref for Handle<T> {
         //
         // Furthermore, all of the code mentioned here is covered by unit tests,
         // which I've run successfully under Miri.
-        let cell = unsafe { &*self.ptr };
+        let slot = unsafe { &*self.ptr };
 
         // Can only panic, if the object has been reserved, but the reservation
         // was never completed.
-        cell.as_ref()
+        slot.as_ref()
             .expect("Handle references non-existing object")
     }
 }

crates/fj-kernel/src/storage/store.rs

@@ -25,7 +25,10 @@ use std::{marker::PhantomData, sync::Arc};
 
 use parking_lot::RwLock;
 
-use super::{blocks::Blocks, Handle};
+use super::{
+    blocks::{Blocks, Index},
+    Handle,
+};
 
 /// Append-only object storage
 #[derive(Debug)]
@@ -36,16 +39,17 @@ pub struct Store<T> {
 impl<T> Store<T> {
     /// Construct a new instance of `Store`
     pub fn new() -> Self {
-        let block_size = 16384;
-        Self {
-            inner: Arc::new(RwLock::new(Blocks::new(block_size))),
-        }
+        let inner = Arc::new(RwLock::new(StoreInnerInner {
+            blocks: Blocks::new(BLOCK_SIZE),
+        }));
+
+        Self { inner }
     }
 
     /// Insert an object into the store
     pub fn insert(&self, object: T) -> Handle<T> {
-        let mut blocks = self.inner.write();
-        let ptr = blocks.push(object);
+        let mut inner = self.inner.write();
+        let ptr = inner.blocks.push(object);
 
         Handle {
             store: self.inner.clone(),
@@ -70,8 +74,8 @@ impl<T> Store<T> {
     /// [`Handle`]'s ID in the construction of the object, or to create groups
     /// of objects that reference each other through their [`Handle`]s.
     pub fn reserve(&self) -> Reservation<T> {
-        let mut blocks = self.inner.write();
-        let (index, ptr) = blocks.reserve();
+        let mut inner = self.inner.write();
+        let (index, ptr) = inner.blocks.reserve();
 
         Reservation {
             store: self.inner.clone(),
@@ -108,9 +112,9 @@ impl<'a, T: 'a> Iterator for Iter<'a, T> {
     type Item = Handle<T>;
 
     fn next(&mut self) -> Option<Self::Item> {
-        let blocks = self.store.read();
+        let inner = self.store.read();
 
-        let block = blocks.get(self.next_block)?;
+        let block = inner.blocks.get(self.next_block)?;
         let object = block.get(self.next_object);
 
         self.next_object += 1;
@@ -128,10 +132,11 @@ impl<'a, T: 'a> Iterator for Iter<'a, T> {
 /// A reservation of a slot for an object within a [`Store`]
 ///
 /// See [`Store::reserve`].
+#[derive(Debug)]
 pub struct Reservation<T> {
     store: StoreInner<T>,
-    ptr: *mut Option<T>,
-    index: (usize, usize),
+    ptr: *const Option<T>,
+    index: Index,
 }
 
 impl<T> Reservation<T> {
@@ -153,17 +158,24 @@ impl<T> Reservation<T> {
     /// [`Handle`]s you acquired from [`Reservation::handle`] without
     /// limitations.
     pub fn complete(self, object: T) -> Handle<T> {
-        let mut blocks = self.store.write();
-        let ptr = blocks.insert(self.index, object);
+        let mut inner = self.store.write();
+        inner.blocks.insert(self.index, object);
 
         Handle {
             store: self.store.clone(),
-            ptr,
+            ptr: self.ptr,
         }
     }
 }
 
-pub type StoreInner<T> = Arc<RwLock<Blocks<T>>>;
+pub type StoreInner<T> = Arc<RwLock<StoreInnerInner<T>>>;
+
+#[derive(Debug)]
+pub struct StoreInnerInner<T> {
+    blocks: Blocks<T>,
+}
+
+const BLOCK_SIZE: usize = 16384;
 
 #[cfg(test)]
 mod tests {