0.12.0

al8n · Jun 30, 2024 · db0d4de · db0d4de
1 parent 8d21265
commit db0d4de
Show file tree

Hide file tree

Showing 6 changed files with 197 additions and 46 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,11 @@
 # CHANGELOG
 
+## 0.12.0
+
+- Bump `rarena-allocator`'s version
+- Change value callback from `impl FnOnce + Copy` to `impl Fn`
+- Give users back the key offset in the ARENA and the key in value callback
+
 ## 0.11.0
 
 - Refactor and extract lock-free ARENA allocator implementation to [`rarena-allocator`](https://github.com/al8n/rarena) crate.
@@ -79,4 +85,3 @@
 
 ## UNRELEASED
 
-
diff --git a/Cargo.toml b/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "skl"
-version = "0.11.4"
+version = "0.12.0"
 edition = "2021"
 rust-version = "1.56.0"
 repository = "https://github.com/al8n/skl"

diff --git a/src/map.rs b/src/map.rs
@@ -399,8 +399,10 @@ impl<T> Node<T> {
     &self,
     arena: &'a Arena,
     trailer: T,
+    key_offset: u32,
+    key: &'a [u8],
     value_size: u32,
-    f: impl FnOnce(&mut VacantBuffer<'a>) -> Result<(), E>,
+    f: impl Fn(&mut VacantValue<'a>) -> Result<(), E>,
   ) -> Result<(), Either<E, Error>> {
     let mut bytes = arena
       .alloc_aligned_bytes::<T>(value_size)
@@ -409,9 +411,10 @@ impl<T> Node<T> {
     let trailer_offset = bytes.offset();
     let value_offset = trailer_offset + mem::size_of::<T>();
 
-    let mut oval = VacantBuffer::new(value_size as usize, value_offset as u32, unsafe {
+    let buffer = VacantBuffer::new(value_size as usize, value_offset as u32, unsafe {
       arena.get_bytes_mut(value_offset, value_size as usize)
     });
+    let mut oval = VacantValue::new(key_offset, key, buffer);
     f(&mut oval).map_err(Either::Left)?;
 
     let remaining = oval.remaining();
@@ -715,7 +718,7 @@ impl<T, C> SkipMap<T, C> {
     key_size: u32,
     kf: impl FnOnce(&mut VacantBuffer<'a>) -> Result<(), E>,
     value_size: u32,
-    vf: impl FnOnce(&mut VacantBuffer<'a>) -> Result<(), E>,
+    vf: impl Fn(&mut VacantValue<'a>) -> Result<(), E>,
   ) -> Result<(NodePtr<T>, Deallocator), Either<E, Error>> {
     self
       .check_node_size(height, key_size, value_size)
@@ -759,6 +762,8 @@ impl<T, C> SkipMap<T, C> {
         .fill_vacant_value(
           trailer_offset as u32,
           trailer_and_value.capacity() as u32,
+          key_offset as u32,
+          self.arena.get_bytes(key_offset, key_cap),
           value_size,
           value_offset,
           vf,
@@ -897,7 +902,7 @@ impl<T, C> SkipMap<T, C> {
     key_size: u32,
     key_offset: u32,
     value_size: u32,
-    vf: impl FnOnce(&mut VacantBuffer<'a>) -> Result<(), E>,
+    vf: impl Fn(&mut VacantValue<'a>) -> Result<(), E>,
   ) -> Result<(NodePtr<T>, Deallocator), Either<E, Error>> {
     self
       .check_node_size(height, key_size, value_size)
@@ -931,6 +936,8 @@ impl<T, C> SkipMap<T, C> {
         .fill_vacant_value(
           trailer_offset as u32,
           trailer_and_value.capacity() as u32,
+          key_offset,
+          self.arena.get_bytes(key_offset as usize, key_size as usize),
           value_size,
           value_offset,
           vf,
@@ -1026,19 +1033,23 @@ impl<T, C> SkipMap<T, C> {
     Ok((oval.len() as u32, Pointer::new(offset, size)))
   }
 
+  #[allow(clippy::too_many_arguments)]
   #[inline]
   unsafe fn fill_vacant_value<'a, E>(
     &'a self,
     offset: u32,
     size: u32,
+    key_offset: u32,
+    key: &'a [u8],
     value_size: u32,
     value_offset: u32,
-    f: impl FnOnce(&mut VacantBuffer<'a>) -> Result<(), E>,
+    f: impl Fn(&mut VacantValue<'a>) -> Result<(), E>,
   ) -> Result<(u32, Pointer), E> {
     let buf = self
       .arena
       .get_bytes_mut(value_offset as usize, value_size as usize);
-    let mut oval = VacantBuffer::new(value_size as usize, value_offset, buf);
+    let buffer = VacantBuffer::new(value_size as usize, value_offset, buf);
+    let mut oval = VacantValue::new(key_offset, key, buffer);
     if let Err(e) = f(&mut oval) {
       self.arena.dealloc(offset, size);
       return Err(e);
@@ -1151,7 +1162,7 @@ impl<T: Trailer, C> SkipMap<T, C> {
     key: &Key<'a, 'b>,
     trailer: T,
     value_size: u32,
-    f: impl FnOnce(&mut VacantBuffer<'a>) -> Result<(), E>,
+    f: &impl Fn(&mut VacantValue<'a>) -> Result<(), E>,
   ) -> Result<(NodePtr<T>, u32, Deallocator), Either<E, Error>> {
     let height = super::random_height(self.opts.max_height().into());
     let (nd, deallocator) = match key {
@@ -1732,7 +1743,7 @@ impl<T: Trailer, C: Comparator> SkipMap<T, C> {
     trailer: T,
     key: Key<'a, 'b>,
     value_size: u32,
-    f: impl FnOnce(&mut VacantBuffer<'a>) -> Result<(), E> + Copy,
+    f: impl Fn(&mut VacantValue<'a>) -> Result<(), E>,
     success: Ordering,
     failure: Ordering,
     ins: &mut Inserter<T>,
@@ -1746,12 +1757,21 @@ impl<T: Trailer, C: Comparator> SkipMap<T, C> {
       if found {
         let node_ptr = ptr.expect("the NodePtr cannot be `None` when we found");
         let old = VersionedEntryRef::from_node(node_ptr, self);
-
-        key.on_fail(&self.arena);
+        let node = node_ptr.as_ref();
+        key.release(&self.arena);
 
         if upsert {
           return self.upsert(
-            old, node_ptr, &key, trailer, value_size, f, success, failure,
+            old,
+            node_ptr,
+            node.key_offset,
+            node.key_size(),
+            &key,
+            trailer,
+            value_size,
+            &f,
+            success,
+            failure,
           );
         }
 
@@ -1792,10 +1812,11 @@ impl<T: Trailer, C: Comparator> SkipMap<T, C> {
       }
     };
 
-    let (nd, height, mut deallocator) = self.new_node(&k, trailer, value_size, f).map_err(|e| {
-      k.on_fail(&self.arena);
-      e
-    })?;
+    let (nd, height, mut deallocator) =
+      self.new_node(&k, trailer, value_size, &f).map_err(|e| {
+        k.release(&self.arena);
+        e
+      })?;
 
     // 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
@@ -1908,12 +1929,23 @@ impl<T: Trailer, C: Comparator> SkipMap<T, C> {
                   .curr
                   .expect("the current should not be `None` when we found");
                 let old = VersionedEntryRef::from_node(node_ptr, self);
-
-                k.on_fail(&self.arena);
+                let node = node_ptr.as_ref();
+                k.release(&self.arena);
 
                 if upsert {
                   deallocator.dealloc(&self.arena);
-                  return self.upsert(old, node_ptr, &k, trailer, value_size, f, success, failure);
+                  return self.upsert(
+                    old,
+                    node_ptr,
+                    node.key_offset,
+                    node.key_size(),
+                    &k,
+                    trailer,
+                    value_size,
+                    &f,
+                    success,
+                    failure,
+                  );
                 }
 
                 deallocator.dealloc(&self.arena);
@@ -1925,7 +1957,7 @@ impl<T: Trailer, C: Comparator> SkipMap<T, C> {
               }
 
               if let Some(p) = fr.found_key {
-                k.on_fail(&self.arena);
+                k.release(&self.arena);
                 let node = nd.as_mut();
                 node.key_offset = p.offset;
                 node.key_size_and_height = encode_key_size_and_height(p.size, p.height.unwrap());
@@ -1970,17 +2002,26 @@ impl<T: Trailer, C: Comparator> SkipMap<T, C> {
     &'a self,
     old: VersionedEntryRef<'a, T, C>,
     node_ptr: NodePtr<T>,
+    key_offset: u32,
+    key_len: u32,
     key: &Key<'a, 'b>,
     trailer: T,
     value_size: u32,
-    f: impl FnOnce(&mut VacantBuffer<'a>) -> Result<(), E>,
+    f: &impl Fn(&mut VacantValue<'a>) -> Result<(), E>,
     success: Ordering,
     failure: Ordering,
   ) -> Result<UpdateOk<'a, 'b, T, C>, Either<E, Error>> {
     match key {
       Key::Occupied(_) | Key::Vacant(_) | Key::Pointer { .. } => node_ptr
         .as_ref()
-        .set_value(&self.arena, trailer, value_size, f)
+        .set_value(
+          &self.arena,
+          trailer,
+          key_offset,
+          self.arena.get_bytes(key_offset as usize, key_len as usize),
+          value_size,
+          f,
+        )
         .map(|_| Either::Left(if old.is_removed() { None } else { Some(old) })),
       Key::Remove(_) | Key::RemoveVacant(_) | Key::RemovePointer { .. } => {
         let node = node_ptr.as_ref();
@@ -2116,6 +2157,6 @@ const fn decode_key_size_and_height(size: u32) -> (u32, u8) {
 
 #[cold]
 #[inline(never)]
-fn noop<E>(_: &mut VacantBuffer<'_>) -> Result<(), E> {
+fn noop<E>(_: &mut VacantValue<'_>) -> Result<(), E> {
   Ok(())
 }
diff --git a/src/map/api.rs b/src/map/api.rs
@@ -457,7 +457,7 @@ impl<T: Trailer, C: Comparator> SkipMap<T, C> {
       return Err(Error::read_only());
     }
 
-    let copy = |buf: &mut VacantBuffer| {
+    let copy = |buf: &mut VacantValue| {
       let _ = buf.write(value);
       Ok(())
     };
@@ -536,7 +536,7 @@ impl<T: Trailer, C: Comparator> SkipMap<T, C> {
     trailer: T,
     key: &'b [u8],
     value_size: u32,
-    f: impl FnOnce(&mut VacantBuffer<'a>) -> Result<(), E> + Copy,
+    f: impl Fn(&mut VacantValue<'a>) -> Result<(), E>,
   ) -> Result<Option<EntryRef<'a, T, C>>, Either<E, Error>> {
     if self.arena.read_only() {
       return Err(Either::Right(Error::read_only()));
@@ -580,7 +580,7 @@ impl<T: Trailer, C: Comparator> SkipMap<T, C> {
       return Err(Error::read_only());
     }
 
-    let copy = |buf: &mut VacantBuffer| {
+    let copy = |buf: &mut VacantValue| {
       let _ = buf.write(value);
       Ok(())
     };
@@ -660,7 +660,7 @@ impl<T: Trailer, C: Comparator> SkipMap<T, C> {
     trailer: T,
     key: &'b [u8],
     value_size: u32,
-    f: impl FnOnce(&mut VacantBuffer<'a>) -> Result<(), E> + Copy,
+    f: impl Fn(&mut VacantValue<'a>) -> Result<(), E>,
   ) -> Result<Option<EntryRef<'a, T, C>>, Either<E, Error>> {
     if self.arena.read_only() {
       return Err(Either::Right(Error::read_only()));
@@ -742,7 +742,7 @@ impl<T: Trailer, C: Comparator> SkipMap<T, C> {
     key_size: u27,
     key: impl FnOnce(&mut VacantBuffer<'a>) -> Result<(), E>,
     val_size: u32,
-    val: impl FnOnce(&mut VacantBuffer<'a>) -> Result<(), E> + Copy,
+    val: impl Fn(&mut VacantValue<'a>) -> Result<(), E>,
   ) -> Result<Option<EntryRef<'a, T, C>>, Either<E, Error>> {
     let vk = self.fetch_vacant_key(u32::from(key_size), key)?;
 
@@ -820,7 +820,7 @@ impl<T: Trailer, C: Comparator> SkipMap<T, C> {
     key_size: u27,
     key: impl FnOnce(&mut VacantBuffer<'a>) -> Result<(), E>,
     val_size: u32,
-    val: impl FnOnce(&mut VacantBuffer<'a>) -> Result<(), E> + Copy,
+    val: impl Fn(&mut VacantValue<'a>) -> Result<(), E>,
   ) -> Result<Option<EntryRef<'a, T, C>>, Either<E, Error>> {
     let vk = self.fetch_vacant_key(u32::from(key_size), key)?;
 

diff --git a/src/map/tests.rs b/src/map/tests.rs
@@ -153,30 +153,18 @@ fn test_empty_map_anon_unify() {
 fn full_in(l: impl FnOnce(usize) -> SkipMap) {
   let l = l(1000);
   let mut found_arena_full = false;
-
-  let mut full_at = 0;
   for i in 0..100 {
     if let Err(e) = l.get_or_insert(0, &make_int_key(i), &make_value(i)) {
       assert!(matches!(
         e,
         Error::Arena(ArenaError::InsufficientSpace { .. })
       ));
       found_arena_full = true;
-      full_at = i;
       break;
     }
   }
 
   assert!(found_arena_full);
-
-  let e = l
-    .get_or_insert(0, &make_int_key(full_at + 1), &make_value(full_at + 1))
-    .unwrap_err();
-
-  assert!(matches!(
-    e,
-    Error::Arena(ArenaError::InsufficientSpace { .. })
-  ));
 }
 
 #[test]
@@ -1502,8 +1490,7 @@ fn test_concurrent_one_key_map_mut() {
     let open_options = OpenOptions::default()
       .create(Some(ARENA_SIZE as u32))
       .read(true)
-      .write(true)
-      .shrink_on_drop(true);
+      .write(true);
     let map_options = MmapOptions::default();
     concurrent_one_key(Arc::new(
       SkipMap::map_mut(p, open_options, map_options)
@@ -2301,7 +2288,7 @@ fn test_reopen_mmap() {
       l.flush().unwrap();
     }
 
-    let open_options = OpenOptions::default().read(true).shrink_on_drop(true);
+    let open_options = OpenOptions::default().read(true);
     let map_options = MmapOptions::default();
     let l = SkipMap::<u64>::map(&p, open_options, map_options, 0).unwrap();
     assert_eq!(1000, l.len());