Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix miri test #25

Closed
wants to merge 2 commits into from
Closed

fix miri test #25

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
db0d4de
0.12.0
al8n Jun 30, 2024
586cb38
fix miri test
al8n Jul 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -79,4 +85,3 @@

## UNRELEASED


2 changes: 1 addition & 1 deletion Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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"
Expand Down
87 changes: 62 additions & 25 deletions src/map.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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();
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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,
node_ref.get_key(&self.arena),
value_size,
value_offset,
vf,
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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,
node_ref.get_key(&self.arena),
value_size,
value_offset,
vf,
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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>,
Expand All @@ -1746,12 +1757,20 @@ 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,
&key,
trailer,
value_size,
&f,
success,
failure,
);
}

Expand Down Expand Up @@ -1792,10 +1811,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
Expand Down Expand Up @@ -1908,12 +1928,22 @@ 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,
&k,
trailer,
value_size,
&f,
success,
failure,
);
}

deallocator.dealloc(&self.arena);
Expand All @@ -1925,7 +1955,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());
Expand Down Expand Up @@ -1970,20 +2000,27 @@ impl<T: Trailer, C: Comparator> SkipMap<T, C> {
&'a self,
old: VersionedEntryRef<'a, T, C>,
node_ptr: NodePtr<T>,
key_offset: 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>> {
let node = node_ptr.as_ref();
match key {
Key::Occupied(_) | Key::Vacant(_) | Key::Pointer { .. } => node_ptr
.as_ref()
.set_value(&self.arena, trailer, value_size, f)
Key::Occupied(_) | Key::Vacant(_) | Key::Pointer { .. } => node
.set_value(
&self.arena,
trailer,
key_offset,
node.get_key(&self.arena),
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();
let key = node.get_key(&self.arena);
match node.clear_value(&self.arena, success, failure) {
Ok(_) => Ok(Either::Left(None)),
Expand Down Expand Up @@ -2116,6 +2153,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(())
}
12 changes: 6 additions & 6 deletions src/map/api.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
};
Expand Down Expand Up @@ -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()));
Expand Down Expand Up @@ -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(())
};
Expand Down Expand Up @@ -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()));
Expand Down Expand Up @@ -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)?;

Expand Down Expand Up @@ -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)?;

Expand Down
17 changes: 2 additions & 15 deletions src/map/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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());
Expand Down
Loading
Loading