From 426815e9aa9759a42a14ef7cf2041f5379fb107a Mon Sep 17 00:00:00 2001 From: chrysn Date: Wed, 29 Apr 2020 09:40:37 +0200 Subject: [PATCH 1/4] Split RawSlots and Slots RawSlots operate purely on unprotecetd indices, Slots gives all the protection around it. --- src/lib.rs | 109 ++++++++++++++++++++++++++++++++++------------------- 1 file changed, 71 insertions(+), 38 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 494f906..dbe577a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -95,10 +95,8 @@ enum EntryInner { // Data type that stores values and returns a key that can be used to manipulate // the stored values. // Values can be read by anyone but can only be modified using the key. -pub struct Slots +pub struct RawSlots where N: ArrayLength> + Unsigned { - #[cfg(feature = "verify_owner")] - id: usize, items: GenericArray, N>, // Could be optimized by making it just usize and relying on free_count to determine its // validity @@ -106,6 +104,13 @@ pub struct Slots free_count: usize } +pub struct Slots +where N: ArrayLength> + Unsigned { + #[cfg(feature = "verify_owner")] + id: usize, + raw: RawSlots +} + #[cfg(feature = "verify_owner")] fn new_instance_id() -> usize { static COUNTER: AtomicUsize = AtomicUsize::new(0); @@ -113,29 +118,18 @@ fn new_instance_id() -> usize { COUNTER.fetch_add(1, Ordering::Relaxed) } -impl Slots +impl RawSlots where N: ArrayLength> + Unsigned { pub fn new() -> Self { let size = N::to_usize(); Self { - #[cfg(feature = "verify_owner")] - id: new_instance_id(), items: GenericArray::generate(|i| Entry(i.checked_sub(1).map(EntryInner::EmptyNext).unwrap_or(EntryInner::EmptyLast))), next_free: size.checked_sub(1), free_count: size } } - #[cfg(feature = "verify_owner")] - fn verify_key(&self, key: &Key) { - assert!(key.owner_id == self.id, "Key used in wrong instance"); - } - - #[cfg(not(feature = "verify_owner"))] - fn verify_key(&self, _key: &Key) { - } - pub fn capacity(&self) -> usize { N::to_usize() } @@ -164,49 +158,88 @@ impl Slots Some(index) } - pub fn store(&mut self, item: IT) -> Result, IT> { + pub fn store(&mut self, item: IT) -> Result { match self.alloc() { Some(i) => { self.items[i] = Entry(EntryInner::Used(item)); - Ok(Key::new(self, i)) + Ok(i) } None => Err(item) } } - pub fn take(&mut self, key: Key) -> IT { - self.verify_key(&key); - - let taken = core::mem::replace(&mut self.items[key.index], Entry(EntryInner::EmptyLast)); - self.free(key.index); + pub fn take(&mut self, key: usize) -> Option { + let taken = core::mem::replace(&mut self.items[key], Entry(EntryInner::EmptyLast)); + self.free(key); match taken.0 { - EntryInner::Used(item) => item, - _ => unreachable!("Invalid key") + EntryInner::Used(item) => Some(item), + _ => None } } - pub fn read(&self, key: &Key, function: F) -> T where F: FnOnce(&IT) -> T { - self.verify_key(&key); - - match self.try_read(key.index, function) { - Some(t) => t, - None => unreachable!("Invalid key") + pub fn read(&self, key: usize, function: F) -> Option where F: FnOnce(&IT) -> T { + match &self.items[key].0 { + EntryInner::Used(item) => Some(function(&item)), + _ => None } } - pub fn try_read(&self, key: usize, function: F) -> Option where F: FnOnce(&IT) -> T { - match &self.items[key].0 { - EntryInner::Used(item) => Some(function(&item)), + pub fn modify(&mut self, key: usize, function: F) -> Option where F: FnOnce(&mut IT) -> T { + match self.items[key].0 { + EntryInner::Used(ref mut item) => Some(function(item)), _ => None } } +} - pub fn modify(&mut self, key: &Key, function: F) -> T where F: FnOnce(&mut IT) -> T { - self.verify_key(&key); +impl Slots + where N: ArrayLength> + Unsigned { - match self.items[key.index].0 { - EntryInner::Used(ref mut item) => function(item), - _ => unreachable!("Invalid key") + pub fn new() -> Self { + Self { + #[cfg(feature = "verify_owner")] + id: new_instance_id(), + raw: RawSlots::new(), } } + + #[cfg(feature = "verify_owner")] + fn verify_key(&self, key: &Key) { + assert!(key.owner_id == self.id, "Key used in wrong instance"); + } + + #[cfg(not(feature = "verify_owner"))] + fn verify_key(&self, _key: &Key) { + } + + pub fn capacity(&self) -> usize { + self.raw.capacity() + } + + pub fn count(&self) -> usize { + self.raw.count() + } + + pub fn store(&mut self, item: IT) -> Result, IT> { + self.raw.store(item).map(|id| Key::new(self, id)) + } + + pub fn take(&mut self, key: Key) -> IT { + self.verify_key(&key); + self.raw.take(key.index()).expect("Invalid key") + } + + pub fn read(&self, key: &Key, function: F) -> T where F: FnOnce(&IT) -> T { + self.verify_key(&key); + self.raw.read(key.index(), function).expect("Invalid key") + } + + pub fn try_read(&self, key: usize, function: F) -> Option where F: FnOnce(&IT) -> T { + self.raw.read(key, function) + } + + pub fn modify(&mut self, key: &Key, function: F) -> T where F: FnOnce(&mut IT) -> T { + self.verify_key(&key); + self.raw.modify(key.index(), function).expect("Invalid key") + } } From ec603ebce60e185afa2719ce9eec9b30c9987376 Mon Sep 17 00:00:00 2001 From: chrysn Date: Thu, 30 Apr 2020 14:43:53 +0200 Subject: [PATCH 2/4] Add rustdoc documentation --- src/lib.rs | 95 +++++++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 83 insertions(+), 12 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index dbe577a..ed0eacd 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -59,6 +59,16 @@ pub use generic_array::ArrayLength; use generic_array::typenum::Unsigned; +/// Access key to a Slots instance +/// +/// Keys must only be used with the Slots they were generated from; using them for any other access +/// is a logic error and results in immediate or later panics from the Slot's functions. Erroneous +/// use is largely caught by type constraints; additional runtime constraints can be enabled using +/// the `verify_owner` feature. +/// +/// Unless `verify_owner` is used, a Key is pointer-sized. +/// +/// Keys can only be created by a Slot's [`store`](struct.Slots.html#method.store) method. pub struct Key { #[cfg(feature = "verify_owner")] owner_id: usize, @@ -79,11 +89,31 @@ impl Key } } + /// The underlying index of the key + /// + /// A key's index can be used in fallible access using the + /// [`try_read()`](struct.Slots.html#method.try_read) method. pub fn index(&self) -> usize { self.index } } +/// Internal element of `RawSlots` instances +/// +/// This struct is not expected to be used by code outside the slots crate, except to define +/// suitable array sizes that are `ArrayLength>` as required by the +/// [`RawSlots`](struct.RawSlots.html) and [`Slots`](struct.Slots.html) generics +/// for usages that are generic over the length of the used slots: +/// +/// ``` +/// # use slots::*; +/// fn examine(slots: &Slots, keys: &[Key]) +/// where +/// N: slots::ArrayLength>, +/// { +/// unimplemented!(); +/// } +/// ``` pub struct Entry(EntryInner); enum EntryInner { @@ -92,9 +122,15 @@ enum EntryInner { EmptyLast } -// Data type that stores values and returns a key that can be used to manipulate -// the stored values. -// Values can be read by anyone but can only be modified using the key. +/// An unchecked slab allocator with predetermined size +/// +/// The allocator deals out usize handles that can be used to access the data later through a +/// (shared or mutable) reference to the `RawSlots`. All access is fallible, as the handles can be +/// arbitrarily created. +/// +/// It is up to slots' users to ensure that the item they intend to access is still identified by +/// that handle, especially as it can have been removed in the meantime, and the handle replaced by +/// a different object. pub struct RawSlots where N: ArrayLength> + Unsigned { items: GenericArray, N>, @@ -104,6 +140,12 @@ pub struct RawSlots free_count: usize } +/// A type-checked slab allocator with predetermined size +/// +/// The allocator deals out [`Key`](struct.Key.html) objects that can be used to access the data +/// later through a (shared or mutable) reference to the Slots. By the interface's design, access +/// is guaranteed to succeed, as the keys are unclonable and consumed to remove an item from the +/// Slots instance. pub struct Slots where N: ArrayLength> + Unsigned { #[cfg(feature = "verify_owner")] @@ -120,6 +162,7 @@ fn new_instance_id() -> usize { impl RawSlots where N: ArrayLength> + Unsigned { + /// Create an empty raw slot allocator of size `N`. pub fn new() -> Self { let size = N::to_usize(); @@ -158,6 +201,10 @@ impl RawSlots Some(index) } + /// Put an item into a free slot + /// + /// This returns an access index for the stored data in the success case, or hands the unstored + /// item back in case of an error. pub fn store(&mut self, item: IT) -> Result { match self.alloc() { Some(i) => { @@ -168,24 +215,31 @@ impl RawSlots } } - pub fn take(&mut self, key: usize) -> Option { - let taken = core::mem::replace(&mut self.items[key], Entry(EntryInner::EmptyLast)); - self.free(key); + /// Move an item out of a slot, if it exists + pub fn take(&mut self, index: usize) -> Option { + let taken = core::mem::replace(&mut self.items[index], Entry(EntryInner::EmptyLast)); + self.free(index); match taken.0 { EntryInner::Used(item) => Some(item), _ => None } } - pub fn read(&self, key: usize, function: F) -> Option where F: FnOnce(&IT) -> T { - match &self.items[key].0 { + /// Provide immutable access to an item + /// + /// The callback is only run if the given index is currently valid. + pub fn read(&self, index: usize, function: F) -> Option where F: FnOnce(&IT) -> T { + match &self.items[index].0 { EntryInner::Used(item) => Some(function(&item)), _ => None } } - pub fn modify(&mut self, key: usize, function: F) -> Option where F: FnOnce(&mut IT) -> T { - match self.items[key].0 { + /// Provide mutable access to an item + // + /// The callback is only run if the given index is currently valid. + pub fn modify(&mut self, index: usize, function: F) -> Option where F: FnOnce(&mut IT) -> T { + match self.items[index].0 { EntryInner::Used(ref mut item) => Some(function(item)), _ => None } @@ -195,6 +249,10 @@ impl RawSlots impl Slots where N: ArrayLength> + Unsigned { + /// Create an empty slot allocator of size `N`. + /// + /// If the `verify_owner` feature is enabled, it will carry a new unique (except for + /// wraparounds) ID which it shares with its keys. pub fn new() -> Self { Self { #[cfg(feature = "verify_owner")] @@ -220,24 +278,37 @@ impl Slots self.raw.count() } + /// Put an item into a free slot + /// + /// This returns an access index for the stored data in the success case, or hands the unstored + /// item back in case of an error. pub fn store(&mut self, item: IT) -> Result, IT> { self.raw.store(item).map(|id| Key::new(self, id)) } + /// Move an item out of its slot pub fn take(&mut self, key: Key) -> IT { self.verify_key(&key); self.raw.take(key.index()).expect("Invalid key") } + /// Provide immutable access to an item pub fn read(&self, key: &Key, function: F) -> T where F: FnOnce(&IT) -> T { self.verify_key(&key); self.raw.read(key.index(), function).expect("Invalid key") } - pub fn try_read(&self, key: usize, function: F) -> Option where F: FnOnce(&IT) -> T { - self.raw.read(key, function) + /// Opportunistic immutable access to an item by its index + /// + /// A suitable index can be generated from a [`Key`](struct.Key.html) through its + /// [`index()`](struct.Key.html#method.index) method; unlike the regular access, this can fail if + /// the element has been removed (in which case the function is not run at all), or might have + /// been replaced by a completely unrelated element inbetween. + pub fn try_read(&self, index: usize, function: F) -> Option where F: FnOnce(&IT) -> T { + self.raw.read(index, function) } + /// Provide mutable access to an item pub fn modify(&mut self, key: &Key, function: F) -> T where F: FnOnce(&mut IT) -> T { self.verify_key(&key); self.raw.modify(key.index(), function).expect("Invalid key") From 2ba1722692953d8792aaa80e6ee9ff4dfa377c52 Mon Sep 17 00:00:00 2001 From: chrysn Date: Thu, 30 Apr 2020 15:56:45 +0200 Subject: [PATCH 3/4] Add a SlotMap implementation --- src/lib.rs | 3 ++ src/slotmap.rs | 118 +++++++++++++++++++++++++++++++++++++++++++++++ tests/slotmap.rs | 29 ++++++++++++ 3 files changed, 150 insertions(+) create mode 100644 src/slotmap.rs create mode 100644 tests/slotmap.rs diff --git a/src/lib.rs b/src/lib.rs index ed0eacd..1c3a615 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -57,6 +57,9 @@ use generic_array::{GenericArray, sequence::GenericSequence}; pub use generic_array::typenum::consts; pub use generic_array::ArrayLength; +mod slotmap; +pub use slotmap::SlotMap; + use generic_array::typenum::Unsigned; /// Access key to a Slots instance diff --git a/src/slotmap.rs b/src/slotmap.rs new file mode 100644 index 0000000..ace1c5c --- /dev/null +++ b/src/slotmap.rs @@ -0,0 +1,118 @@ +use crate::{RawSlots, ArrayLength, Entry, Unsigned}; + +/// A SlotMap on a fixed size array with compact slot identifiers +/// +/// A SlotMap is a pool allocator whose handles carry additional generation information. +/// +/// This implementation inherits the properties of [`RawSlots`](struct.RawSlots.html) (in +/// particular, unlike other slot maps, it does not aim for contiguous allocation of items or +/// growability). +/// +/// It stores the current generation as part of the slot map rather than the slot +/// (for simplicity of implementation; per-slot generations that persist data in the slot can not +/// be built on `RawSlots`), and combines index and generation into a single unsigned handler +/// created by concatenating the truncated generation counter by the width of the actuallly +/// required index. +/// +/// It is a drop-in replacement for `RawSlots` with some performance penalty (storing an additional +/// usize per entry and map, and additional checks at runtime) with a largely reduced risk of an +/// old key accidentally matching a new object. +pub struct SlotMap +where + N: ArrayLength> + Unsigned +{ + raw: RawSlots<(usize, IT), N>, + generation: usize, +} + +impl SlotMap +where + N: ArrayLength> + Unsigned +{ + // All those pubs could be shared in an interface with RawSlots -- same API, just less likely + // collisions + + /// Create an empty slot map of size `N`. + pub fn new() -> Self { + Self { + raw: RawSlots::new(), + generation: 0, + } + } + + fn pull_generation(&mut self) -> usize { + self.generation = self.generation.wrapping_add(1); + self.generation + } + + /// Number of bits maximally required for an actual index + // technically const + fn shiftsize() -> u32 { + core::mem::size_of::() as u32 * 8 - (N::to_usize().saturating_sub(1)).leading_zeros() + } + + fn build_handle(&mut self, index: usize) -> usize { + let genpart = self.pull_generation() << Self::shiftsize(); + index | genpart + } + + fn extract_index(handle: usize) -> usize { + // Bits that contain the index + let mask = !(!0 << Self::shiftsize()); + handle & mask + } + + /// Put an item into a free slot + /// + /// This returns an access handle for the stored data in the success case, or hands the unstored + /// item back in case of an error. + pub fn store(&mut self, item: IT) -> Result { + // If we only stored the trimmed generation in the checking field rather than the full + // index, this could be a bit smoother, but the compiler should be able to optimize this + // into a single write access. (And on the other hand, this eases access checks). + match self.raw.store((0, item)) { + Err((_, item)) => Err(item), + Ok(i) => { + let handle = self.build_handle(i); + self.raw.modify(i, |item| item.0 = handle); + Ok(handle) + } + } + } + + /// Move an item out of its slot, if it exists + pub fn take(&mut self, handle: usize) -> Option { + let index = Self::extract_index(handle); + if self.raw.read(index, |&(check, _)| check == handle) == Some(true) { + Some(self.raw.take(index).expect("Item was checked to be present").1) + } else { + None + } + } + + /// Provide immutable access to an item + /// + /// The callback is only run if the given handle matches the currently stored item. + pub fn read(&self, handle: usize, function: F) -> Option where F: FnOnce(&IT) -> T { + self.raw.read(Self::extract_index(handle), |(check, data)| + if check == &handle { + Some(function(&data)) + } else { + None + } + ).flatten() + } + + /// Provide mutable access to an item + // + /// The callback is only run if the given handle matches the currently stored item. + pub fn modify(&mut self, handle: usize, function: F) -> Option where F: FnOnce(&mut IT) -> T { + self.raw.modify(Self::extract_index(handle), |(check, data)| + if check == &handle { + Some(function(data)) + } else { + None + } + ).flatten() + } +} diff --git a/tests/slotmap.rs b/tests/slotmap.rs new file mode 100644 index 0000000..9fa4147 --- /dev/null +++ b/tests/slotmap.rs @@ -0,0 +1,29 @@ +use slots::SlotMap; +use slots::consts::*; + +#[test] +fn key_reuse_triggers() { + let mut slots: SlotMap<_, U8> = SlotMap::new(); + + let k1 = slots.store(5).unwrap(); + let ptr1 = slots.read(k1, |w| w as *const _).unwrap(); + + assert_eq!(slots.take(k1).unwrap(), 5); + + let k2 = slots.store(6).unwrap(); + let ptr2 = slots.read(k2, |w| w as *const _).unwrap(); + + // This test relies on the undocumented but present property that after freeing someting, the + // next allocation goes there right again. + assert_eq!(ptr1, ptr2); + + assert_ne!(k1, k2); + + assert_eq!(None, slots.read(k1, |w| *w)); + + // This, in addition, relies on the current behavior of RawSlots to in the last slot first, and + // on it starting with generation 1. + assert_eq!(k1, 8 | 7); + assert_eq!(k2, 16 | 7); +} + From b61b5a342be8ec160b3c7711a7f2356c45814cdc Mon Sep 17 00:00:00 2001 From: chrysn Date: Thu, 30 Apr 2020 16:18:53 +0200 Subject: [PATCH 4/4] tests: Add a benchmark comparison for slotmap and raw slots --- tests/slotmap.rs | 46 +++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 45 insertions(+), 1 deletion(-) diff --git a/tests/slotmap.rs b/tests/slotmap.rs index 9fa4147..c590bd7 100644 --- a/tests/slotmap.rs +++ b/tests/slotmap.rs @@ -1,6 +1,13 @@ -use slots::SlotMap; +#![warn(soft_unstable)] +#![feature(test)] + +extern crate test; + +use slots::*; use slots::consts::*; +use test::Bencher; + #[test] fn key_reuse_triggers() { let mut slots: SlotMap<_, U8> = SlotMap::new(); @@ -27,3 +34,40 @@ fn key_reuse_triggers() { assert_eq!(k2, 16 | 7); } +struct Thing { + x: [usize; 20], + y: Option, +} + +impl Thing { + fn new() -> Self { + Self { x: [10; 20], y: Some(20) } + } +} + +#[bench] +fn bench_slotmap(b: &mut Bencher) { + b.iter(|| { + let mut s: slots::SlotMap<_, U128> = SlotMap::new(); + + for _ in 0..128 { + let h = s.store(Thing::new()).ok().unwrap(); + s.read(h, |i| test::black_box(i.y)).unwrap(); + } + }) +} + +// This is identical to bench_slotmap except for the type. +// That this becomes *much* faster than slotmap on larger Thing sizes indicates that somewhere, the +// slotmap implementation is flawed in that it moves data around where it should not. +#[bench] +fn bench_rawslots(b: &mut Bencher) { + b.iter(|| { + let mut s: slots::RawSlots<_, U128> = RawSlots::new(); + + for _ in 0..128 { + let h = s.store(Thing::new()).ok().unwrap(); + s.read(h, |i| test::black_box(i.y)).unwrap(); + } + }) +}