Improve entry state pattern

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "skl"
-version = "0.22.4"
+version = "0.22.5"
 edition = "2021"
 rust-version = "1.81.0"
 repository = "https://github.com/al8n/skl"

src/allocator.rs

@@ -108,7 +108,7 @@ mod sealed {
     ) -> Result<(u32, u32), (u32, u32)>;
 
     #[inline]
-    fn is_removed(&self) -> bool {
+    fn is_tombstone(&self) -> bool {
       self.load().1 == Self::REMOVE
     }
   }
@@ -342,8 +342,8 @@ mod sealed {
     }
 
     #[inline]
-    fn is_removed(&self) -> bool {
-      self.value_pointer().is_removed()
+    fn is_tombstone(&self) -> bool {
+      self.value_pointer().is_tombstone()
     }
 
     #[allow(dead_code)]

src/dynamic.rs

@@ -26,75 +26,3 @@ pub type Ascend = dbutils::equivalentor::Ascend<[u8]>;
 
 /// Ascend is a comparator that compares byte slices in ascending order.
 pub type Descend = dbutils::equivalentor::Descend<[u8]>;
-
-/// Value that can be converted from a byte slice.
-pub trait Value<'a>: sealed::Sealed<'a> {}
-
-impl<'a, T> Value<'a> for T where T: sealed::Sealed<'a> {}
-
-mod sealed {
-  pub trait Sealed<'a> {
-    type Ref;
-
-    fn as_ref(&self) -> Self::Ref;
-
-    fn from_value_bytes(src: Option<&'a [u8]>) -> Self
-    where
-      Self: 'a;
-
-    fn is_removed(&self) -> bool;
-
-    fn all_versions(&self) -> bool;
-  }
-
-  impl<'a> Sealed<'a> for Option<&'a [u8]> {
-    type Ref = Self;
-
-    #[inline]
-    fn as_ref(&self) -> Self::Ref {
-      self.as_ref().copied()
-    }
-
-    #[inline]
-    fn from_value_bytes(src: Option<&'a [u8]>) -> Self {
-      src
-    }
-
-    #[inline]
-    fn is_removed(&self) -> bool {
-      self.is_none()
-    }
-
-    #[inline]
-    fn all_versions(&self) -> bool {
-      true
-    }
-  }
-
-  impl<'a> Sealed<'a> for &'a [u8] {
-    type Ref = Self;
-
-    #[inline]
-    fn as_ref(&self) -> Self::Ref {
-      self
-    }
-
-    #[inline]
-    fn from_value_bytes(src: Option<&'a [u8]>) -> Self {
-      match src {
-        Some(v) => v,
-        None => panic!("cannot convert None to Value"),
-      }
-    }
-
-    #[inline]
-    fn is_removed(&self) -> bool {
-      false
-    }
-
-    #[inline]
-    fn all_versions(&self) -> bool {
-      false
-    }
-  }
-}

src/dynamic/list.rs

@@ -10,7 +10,6 @@ use rarena_allocator::Allocator as _;
 
 use crate::{
   allocator::{Allocator, Deallocator, Meta, Node, NodePointer, Pointer, ValuePointer},
-  dynamic::Value,
   encode_key_size_and_height,
   error::Error,
   internal::RefMeta,
@@ -19,7 +18,7 @@ use crate::{
   ref_counter::RefCounter,
   traits::Constructable,
   types::{internal::ValuePointer as ValuePointerType, Height, KeyBuilder, ValueBuilder},
-  FindResult, Header, Inserter, Splice, Version,
+  FindResult, Header, Inserter, MaybeTombstone, Splice, State, Version,
 };
 
 mod entry;
@@ -29,8 +28,8 @@ mod api;
 pub(super) mod iterator;
 
 type UpdateOk<'a, 'b, A, RC, C> = Either<
-  Option<EntryRef<'a, Option<&'a [u8]>, C, A, RC>>,
-  Result<EntryRef<'a, Option<&'a [u8]>, C, A, RC>, EntryRef<'a, Option<&'a [u8]>, C, A, RC>>,
+  Option<EntryRef<'a, MaybeTombstone, C, A, RC>>,
+  Result<EntryRef<'a, MaybeTombstone, C, A, RC>, EntryRef<'a, MaybeTombstone, C, A, RC>>,
 >;
 
 /// A fast, cocnurrent map implementation based on skiplist that supports forward
@@ -315,14 +314,14 @@ where
   C: BytesComparator,
   R: RefCounter,
 {
-  unsafe fn move_to_prev<'a, V>(
+  unsafe fn move_to_prev<'a, S>(
     &'a self,
     nd: &mut <A::Node as Node>::Pointer,
     version: Version,
     contains_key: impl Fn(&[u8]) -> bool,
-  ) -> Option<EntryRef<'a, V, C, A, R>>
+  ) -> Option<EntryRef<'a, S, C, A, R>>
   where
-    V: Value<'a> + 'a,
+    S: State<'a>,
   {
     loop {
       unsafe {
@@ -340,23 +339,24 @@ where
           let pointer = nd.get_value_pointer::<A>();
           let value =
             nd.get_value_by_value_offset(&self.arena, pointer.value_offset, pointer.value_len);
-          let ent = EntryRef::from_node_with_pointer(version, *nd, self, Some(nk), value);
-          return Some(ent.map());
+          let ent =
+            EntryRef::from_node_with_pointer(version, *nd, self, Some(nk), S::from_bytes(value));
+          return Some(ent);
         }
 
         *nd = self.get_prev(*nd, 0);
       }
     }
   }
 
-  unsafe fn move_to_prev_maximum_version<'a, V>(
+  unsafe fn move_to_prev_maximum_version<'a, S>(
     &'a self,
     nd: &mut <A::Node as Node>::Pointer,
     version: Version,
     contains_key: impl Fn(&[u8]) -> bool,
-  ) -> Option<EntryRef<'a, V, C, A, R>>
+  ) -> Option<EntryRef<'a, S, C, A, R>>
   where
-    V: Value<'a> + 'a,
+    S: State<'a>,
   {
     loop {
       unsafe {
@@ -373,16 +373,22 @@ where
 
         if prev.is_null() || prev.offset() == self.head.offset() {
           // prev is null or the head, we should try to see if we can return the current node.
-          if !nd.is_removed() {
+          if !nd.is_tombstone() {
             // the current node is valid, we should return it.
             let nk = nd.get_key(&self.arena);
 
             if contains_key(nk) {
               let pointer = nd.get_value_pointer::<A>();
               let value =
                 nd.get_value_by_value_offset(&self.arena, pointer.value_offset, pointer.value_len);
-              let ent = EntryRef::from_node_with_pointer(version, *nd, self, Some(nk), value);
-              return Some(ent.map());
+              let ent = EntryRef::from_node_with_pointer(
+                version,
+                *nd,
+                self,
+                Some(nk),
+                S::from_bytes(value),
+              );
+              return Some(ent);
             }
           }
 
@@ -395,12 +401,13 @@ where
         if prev.version() > version || nd.get_key(&self.arena).ne(prev.get_key(&self.arena)) {
           let nk = nd.get_key(&self.arena);
 
-          if !nd.is_removed() && contains_key(nk) {
+          if !nd.is_tombstone() && contains_key(nk) {
             let pointer = nd.get_value_pointer::<A>();
             let value =
               nd.get_value_by_value_offset(&self.arena, pointer.value_offset, pointer.value_len);
-            let ent = EntryRef::from_node_with_pointer(version, *nd, self, Some(nk), value);
-            return Some(ent.map());
+            let ent =
+              EntryRef::from_node_with_pointer(version, *nd, self, Some(nk), S::from_bytes(value));
+            return Some(ent);
           }
         }
 
@@ -409,14 +416,14 @@ where
     }
   }
 
-  unsafe fn move_to_next<'a, V>(
+  unsafe fn move_to_next<'a, S>(
     &'a self,
     nd: &mut <A::Node as Node>::Pointer,
     version: Version,
     contains_key: impl Fn(&[u8]) -> bool,
-  ) -> Option<EntryRef<'a, V, C, A, R>>
+  ) -> Option<EntryRef<'a, S, C, A, R>>
   where
-    V: Value<'a> + 'a,
+    S: State<'a>,
   {
     loop {
       unsafe {
@@ -434,23 +441,24 @@ where
           let pointer = nd.get_value_pointer::<A>();
           let value =
             nd.get_value_by_value_offset(&self.arena, pointer.value_offset, pointer.value_len);
-          let ent = EntryRef::from_node_with_pointer(version, *nd, self, Some(nk), value);
-          return Some(ent.map());
+          let ent =
+            EntryRef::from_node_with_pointer(version, *nd, self, Some(nk), S::from_bytes(value));
+          return Some(ent);
         }
 
         *nd = self.get_next(*nd, 0);
       }
     }
   }
 
-  unsafe fn move_to_next_maximum_version<'a, V>(
+  unsafe fn move_to_next_maximum_version<'a, S>(
     &'a self,
     nd: &mut <A::Node as Node>::Pointer,
     version: Version,
     contains_key: impl Fn(&[u8]) -> bool,
-  ) -> Option<EntryRef<'a, V, C, A, R>>
+  ) -> Option<EntryRef<'a, S, C, A, R>>
   where
-    V: Value<'a> + 'a,
+    S: State<'a>,
   {
     loop {
       unsafe {
@@ -466,7 +474,7 @@ where
         }
 
         // if the entry with largest version is removed, we should skip this key.
-        if nd.is_removed() {
+        if nd.is_tombstone() {
           let mut next = self.get_next(*nd, 0);
           let curr_key = nd.get_key(&self.arena);
           loop {
@@ -491,8 +499,9 @@ where
           let pointer = nd.get_value_pointer::<A>();
           let value =
             nd.get_value_by_value_offset(&self.arena, pointer.value_offset, pointer.value_len);
-          let ent = EntryRef::from_node_with_pointer(version, *nd, self, Some(nk), value);
-          return Some(ent.map());
+          let ent =
+            EntryRef::from_node_with_pointer(version, *nd, self, Some(nk), S::from_bytes(value));
+          return Some(ent);
         }
 
         *nd = self.get_next(*nd, 0);
@@ -878,7 +887,7 @@ where
           );
         }
 
-        return Ok(Either::Left(if old.is_removed() {
+        return Ok(Either::Left(if old.is_tombstone() {
           None
         } else {
           Some(old)
@@ -913,7 +922,7 @@ where
         k.on_fail(&self.arena);
       })?;
 
-    let is_removed = unsafe { unlinked_node.get_value(&self.arena).is_none() };
+    let is_tombstone = unsafe { unlinked_node.get_value(&self.arena).is_none() };
 
     // We always insert from the base level and up. After you add a node in base
     // level, we cannot create a node in the level above because it would have
@@ -1047,7 +1056,7 @@ where
                       version,
                       old,
                       node_ptr,
-                      &if is_removed {
+                      &if is_tombstone {
                         Key::<A>::remove_pointer(&self.arena, fr.found_key.unwrap())
                       } else {
                         Key::<A>::pointer(&self.arena, fr.found_key.unwrap())
@@ -1061,7 +1070,7 @@ where
                 }
 
                 deallocator.dealloc(&self.arena);
-                return Ok(Either::Left(if old.is_removed() {
+                return Ok(Either::Left(if old.is_tombstone() {
                   None
                 } else {
                   Some(old)
@@ -1112,7 +1121,7 @@ where
   unsafe fn upsert_value<'a, 'b: 'a>(
     &'a self,
     version: Version,
-    old: EntryRef<'a, Option<&'a [u8]>, C, A, R>,
+    old: EntryRef<'a, MaybeTombstone, C, A, R>,
     old_node: <A::Node as Node>::Pointer,
     key: &Key<'a, 'b, A>,
     value_offset: u32,
@@ -1124,7 +1133,7 @@ where
       Key::Occupied(_) | Key::Vacant { .. } | Key::Pointer { .. } => {
         old_node.update_value(&self.arena, value_offset, value_size);
 
-        Ok(Either::Left(if old.is_removed() {
+        Ok(Either::Left(if old.is_tombstone() {
           None
         } else {
           Some(old)
@@ -1153,7 +1162,7 @@ where
   unsafe fn upsert<'a, 'b: 'a, E>(
     &'a self,
     version: Version,
-    old: EntryRef<'a, Option<&'a [u8]>, C, A, R>,
+    old: EntryRef<'a, MaybeTombstone, C, A, R>,
     old_node: <A::Node as Node>::Pointer,
     key: &Key<'a, 'b, A>,
     value_builder: Option<ValueBuilder<impl FnOnce(&mut VacantBuffer<'a>) -> Result<usize, E>>>,
@@ -1164,7 +1173,7 @@ where
       Key::Occupied(_) | Key::Vacant { .. } | Key::Pointer { .. } => self
         .arena
         .allocate_and_update_value(&old_node, value_builder.unwrap())
-        .map(|_| Either::Left(if old.is_removed() { None } else { Some(old) })),
+        .map(|_| Either::Left(if old.is_tombstone() { None } else { Some(old) })),
       Key::Remove(_) | Key::RemoveVacant { .. } | Key::RemovePointer { .. } => {
         match old_node.clear_value(&self.arena, success, failure) {
           Ok(_) => Ok(Either::Left(None)),