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.

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

0.12.0 #24

Merged
merged 4 commits into from
Jun 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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
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
85 changes: 63 additions & 22 deletions src/map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -399,8 +399,10 @@
&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 @@
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 @@
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 @@
.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,
Expand Down Expand Up @@ -897,7 +902,7 @@
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 @@
.fill_vacant_value(
trailer_offset as u32,
trailer_and_value.capacity() as u32,
key_offset,

Check warning on line 939 in src/map.rs

View check run for this annotation

Codecov / codecov/patch

src/map.rs#L939

Added line #L939 was not covered by tests
self.arena.get_bytes(key_offset as usize, key_size as usize),
value_size,
value_offset,
vf,
Expand Down Expand Up @@ -1026,19 +1033,23 @@
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 @@
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 @@
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,21 @@
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,

Check warning on line 1766 in src/map.rs

View check run for this annotation

Codecov / codecov/patch

src/map.rs#L1766

Added line #L1766 was not covered by tests
node.key_offset,
node.key_size(),
&key,

Check warning on line 1769 in src/map.rs

View check run for this annotation

Codecov / codecov/patch

src/map.rs#L1769

Added line #L1769 was not covered by tests
trailer,
value_size,
&f,
success,
failure,

Check warning on line 1774 in src/map.rs

View check run for this annotation

Codecov / codecov/patch

src/map.rs#L1771-L1774

Added lines #L1771 - L1774 were not covered by tests
);
}

Expand Down Expand Up @@ -1792,10 +1812,11 @@
}
};

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| {

Check warning on line 1816 in src/map.rs

View check run for this annotation

Codecov / codecov/patch

src/map.rs#L1816

Added line #L1816 was not covered by tests
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 +1929,23 @@
.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);

Check warning on line 1933 in src/map.rs

View check run for this annotation

Codecov / codecov/patch

src/map.rs#L1932-L1933

Added lines #L1932 - L1933 were not covered by tests

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,

Check warning on line 1947 in src/map.rs

View check run for this annotation

Codecov / codecov/patch

src/map.rs#L1937-L1947

Added lines #L1937 - L1947 were not covered by tests
);
}

deallocator.dealloc(&self.arena);
Expand All @@ -1925,7 +1957,7 @@
}

if let Some(p) = fr.found_key {
k.on_fail(&self.arena);
k.release(&self.arena);

Check warning on line 1960 in src/map.rs

View check run for this annotation

Codecov / codecov/patch

src/map.rs#L1960

Added line #L1960 was not covered by tests
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,17 +2002,26 @@
&'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,

Check warning on line 2020 in src/map.rs

View check run for this annotation

Codecov / codecov/patch

src/map.rs#L2019-L2020

Added lines #L2019 - L2020 were not covered by tests
self.arena.get_bytes(key_offset as usize, key_len as usize),
value_size,
f,

Check warning on line 2023 in src/map.rs

View check run for this annotation

Codecov / codecov/patch

src/map.rs#L2022-L2023

Added lines #L2022 - L2023 were not covered by tests
)
.map(|_| Either::Left(if old.is_removed() { None } else { Some(old) })),
Key::Remove(_) | Key::RemoveVacant(_) | Key::RemovePointer { .. } => {
let node = node_ptr.as_ref();
Expand Down Expand Up @@ -2116,6 +2157,6 @@

#[cold]
#[inline(never)]
fn noop<E>(_: &mut VacantBuffer<'_>) -> Result<(), E> {
fn noop<E>(_: &mut VacantValue<'_>) -> Result<(), E> {

Check warning on line 2160 in src/map.rs

View check run for this annotation

Codecov / codecov/patch

src/map.rs#L2160

Added line #L2160 was not covered by tests
Ok(())
}
12 changes: 6 additions & 6 deletions src/map/api.rs
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
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