diff --git a/crates/fj-kernel/src/algorithms/approx/curve.rs b/crates/fj-kernel/src/algorithms/approx/curve.rs index 2ef17650e..cb107956a 100644 --- a/crates/fj-kernel/src/algorithms/approx/curve.rs +++ b/crates/fj-kernel/src/algorithms/approx/curve.rs @@ -14,7 +14,7 @@ use std::collections::BTreeMap; use crate::{ objects::{Curve, GlobalCurve}, path::{GlobalPath, SurfacePath}, - stores::Handle, + stores::{Handle, ObjectId}, }; use super::{path::RangeOnPath, Approx, ApproxPoint, Tolerance}; @@ -30,12 +30,12 @@ impl Approx for (&Curve, RangeOnPath) { ) -> Self::Approximation { let (curve, range) = self; - let cache_key = (curve.global_form().clone(), range); - let global_curve_approx = match cache.get(cache_key.clone()) { + let global_curve = curve.global_form().clone(); + let global_curve_approx = match cache.get(global_curve.clone(), range) { Some(approx) => approx, None => { let approx = approx_global_curve(curve, range, tolerance); - cache.insert(cache_key, approx) + cache.insert(global_curve, range, approx) } }; @@ -152,7 +152,7 @@ impl CurveApprox { /// A cache for results of an approximation #[derive(Default)] pub struct CurveCache { - inner: BTreeMap<(Handle, RangeOnPath), GlobalCurveApprox>, + inner: BTreeMap<(ObjectId, RangeOnPath), GlobalCurveApprox>, } impl CurveCache { @@ -164,19 +164,21 @@ impl CurveCache { /// Insert the approximation of a [`GlobalCurve`] pub fn insert( &mut self, - key: (Handle, RangeOnPath), + handle: Handle, + range: RangeOnPath, approx: GlobalCurveApprox, ) -> GlobalCurveApprox { - self.inner.insert(key, approx.clone()); + self.inner.insert((handle.id(), range), approx.clone()); approx } /// Access the approximation for the given [`GlobalCurve`], if available pub fn get( &self, - key: (Handle, RangeOnPath), + handle: Handle, + range: RangeOnPath, ) -> Option { - self.inner.get(&key).cloned() + self.inner.get(&(handle.id(), range)).cloned() } } diff --git a/crates/fj-kernel/src/stores/blocks.rs b/crates/fj-kernel/src/stores/blocks.rs new file mode 100644 index 000000000..a6f339c84 --- /dev/null +++ b/crates/fj-kernel/src/stores/blocks.rs @@ -0,0 +1,137 @@ +use std::iter; + +#[derive(Debug)] +pub struct Blocks { + inner: Vec>, + block_size: usize, +} + +impl Blocks { + pub fn new(block_size: usize) -> Self { + Self { + inner: Vec::new(), + block_size, + } + } + + pub fn push(&mut self, object: T) -> *const Option { + let (index, _) = self.reserve(); + self.insert(index, object) + } + + pub fn reserve(&mut self) -> ((usize, usize), *mut Option) { + let mut current_block = match self.inner.pop() { + Some(block) => block, + None => Block::new(self.block_size), + }; + + let ret = loop { + match current_block.reserve() { + Ok((object_index, ptr)) => { + let block_index = self.inner.len(); + break ((block_index, object_index), ptr); + } + Err(()) => { + // Block is full. Need to create a new one and retry. + self.inner.push(current_block); + current_block = Block::new(self.block_size); + } + } + }; + + self.inner.push(current_block); + + ret + } + + pub fn insert( + &mut self, + (block_index, object_index): (usize, usize), + object: T, + ) -> *const Option { + let block = &mut self.inner[block_index]; + block.insert(object_index, object) + } + + pub fn get(&self, index: usize) -> Option<&Block> { + self.inner.get(index) + } + + #[cfg(test)] + pub fn iter(&self) -> impl Iterator + '_ { + self.inner.iter().flat_map(|block| block.iter()) + } +} + +#[derive(Debug)] +pub struct Block { + objects: Box<[Option]>, + next: usize, +} + +impl Block { + pub fn new(size: usize) -> Self { + let vec = iter::repeat_with(|| None) + .take(size) + .collect::>>(); + let objects = vec.into_boxed_slice(); + + Self { objects, next: 0 } + } + + pub fn reserve(&mut self) -> Result<(usize, *mut Option), ()> { + if self.next >= self.objects.len() { + return Err(()); + } + + let index = self.next; + let ptr = &mut self.objects[self.next]; + self.next += 1; + + Ok((index, ptr)) + } + + pub fn insert(&mut self, index: usize, object: T) -> *const Option { + self.objects[index] = Some(object); + &self.objects[index] + } + + pub fn get(&self, index: usize) -> &Option { + &self.objects[index] + } + + pub fn len(&self) -> usize { + self.next + } + + #[cfg(test)] + pub fn iter(&self) -> impl Iterator + '_ { + let mut i = 0; + iter::from_fn(move || { + if i >= self.len() { + return None; + } + + let object = self.get(i).as_ref()?; + i += 1; + + Some(object) + }) + } +} + +#[cfg(test)] +mod tests { + use super::Blocks; + + #[test] + fn push() { + let mut blocks = Blocks::new(1); + + blocks.push(0); + blocks.push(1); + + let objects = blocks.iter().copied().collect::>(); + assert_eq!(objects, [0, 1]); + } +} diff --git a/crates/fj-kernel/src/stores/handle.rs b/crates/fj-kernel/src/stores/handle.rs new file mode 100644 index 000000000..d8a89a9d2 --- /dev/null +++ b/crates/fj-kernel/src/stores/handle.rs @@ -0,0 +1,228 @@ +use std::{any::type_name, fmt, hash::Hash, ops::Deref}; + +use super::store::StoreInner; + +/// A handle for an object +/// +/// You can get an instance of `Handle` by inserting an object into a store. A +/// handle dereferences to the object it points to, via its [`Deref`] +/// implementation. +/// +/// # Equality and Identity +/// +/// Equality of `Handle`s is defined via the objects they reference. If those +/// objects are equal, the `Handle`s are considered equal. +/// +/// This is distinct from the *identity* of the referenced objects. Two objects +/// might be equal, but they might be have been created at different times, for +/// different reasons, and thus live in different slots in the storage. This is +/// a relevant distinction when validating objects, as equal but not identical +/// objects might be a sign of a bug. +/// +/// You can compare the identity of two objects through their `Handle`s, by +/// comparing the values returned by [`Handle::id`]. +pub struct Handle { + pub(super) store: StoreInner, + pub(super) ptr: *const Option, +} + +impl Handle { + /// Access this pointer's unique id + pub fn id(&self) -> ObjectId { + ObjectId(self.ptr as u64) + } + + /// Return a clone of the object this handle refers to + pub fn clone_object(&self) -> T + where + T: Clone, + { + self.deref().clone() + } +} + +impl Deref for Handle { + type Target = T; + + fn deref(&self) -> &Self::Target { + // `Handle` keeps a reference to `StoreInner`. Since that is an `Arc` + // under the hood, we know that as long as an instance of `Handle` + // exists, the `StoreInner` its data lives in is still alive. Even if + // the `Store` was dropped. + // + // The `Store` API ensures two things: + // + // 1. That no `Handle` is ever created, until the object it references + // has at least been reserved. + // 2. That the memory objects live in is never deallocated. + // + // That means that as long as a `Handle` exists, the object is + // references has at least been reserved, and has not been deallocated. + // + // Given all this, we know that the following must be true: + // + // - The pointer is not null. + // - The pointer is properly aligned. + // - The pointer is dereferenceable. + // - The pointer points to an initialized instance of `T`. + // + // Further, there is no way to (safely) get a `&mut` reference to any + // object in a `Store`/`Block`. So we know that the aliasing rules for + // the reference we return here are enforced. + // + // Furthermore, all of the code mentioned here is covered by unit tests, + // which I've run successfully run under Miri. + let cell = unsafe { &*self.ptr }; + + // Can only happen, if the object has been reserved, but the reservation + // was never completed. + cell.as_ref() + .expect("Handle references non-existing object") + } +} + +impl Clone for Handle { + fn clone(&self) -> Self { + Self { + store: self.store.clone(), + ptr: self.ptr, + } + } +} + +impl Eq for Handle where T: Eq {} + +impl PartialEq for Handle +where + T: PartialEq, +{ + fn eq(&self, other: &Self) -> bool { + self.deref().eq(other.deref()) + } +} + +impl Hash for Handle +where + T: Hash, +{ + fn hash(&self, state: &mut H) { + self.deref().hash(state) + } +} + +impl Ord for Handle +where + T: Ord, +{ + fn cmp(&self, other: &Self) -> std::cmp::Ordering { + self.deref().cmp(other.deref()) + } +} + +impl PartialOrd for Handle +where + T: PartialOrd, +{ + fn partial_cmp(&self, other: &Self) -> Option { + self.deref().partial_cmp(other.deref()) + } +} + +impl fmt::Debug for Handle { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + let name = { + let type_name = type_name::(); + match type_name.rsplit_once("::") { + Some((_, name)) => name, + None => type_name, + } + }; + let id = self.id().0; + + write!(f, "{name} @ {id:#x}")?; + + Ok(()) + } +} + +unsafe impl Send for Handle {} +unsafe impl Sync for Handle {} + +/// Represents the ID of an object +/// +/// See [`Handle::id`]. +#[derive(Clone, Copy, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] +pub struct ObjectId(u64); + +/// A wrapper around [`Handle`] to define equality based on identity +/// +/// This is a utility type that implements [`Eq`]/[`PartialEq`] and other common +/// traits that are based on those, based on the identity of object that the +/// wrapped handle references. This is useful, if a type of object doesn't +/// implement `Eq`/`PartialEq`, which means handles referencing it won't +/// implement those types either. +/// +/// Typically, if an object doesn't implement [`Eq`]/[`PartialEq`], it will do +/// so for good reason. If you need something that represents the object and +/// implements those missing traits, you might want to be explicit about what +/// you're doing, and access its ID via [`Handle::id`] instead. +/// +/// But if you have a struct that owns a [`Handle`] to such an object, and you +/// want to be able to derive various traits that are not available for the +/// [`Handle`] itself, this wrapper is for you. +pub struct HandleWrapper(pub Handle); + +impl Deref for HandleWrapper { + type Target = Handle; + + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +impl Clone for HandleWrapper { + fn clone(&self) -> Self { + Self(self.0.clone()) + } +} + +impl Eq for HandleWrapper {} + +impl PartialEq for HandleWrapper { + fn eq(&self, other: &Self) -> bool { + self.0.id().eq(&other.0.id()) + } +} + +impl Hash for HandleWrapper { + fn hash(&self, state: &mut H) { + self.0.id().hash(state) + } +} + +impl Ord for HandleWrapper { + fn cmp(&self, other: &Self) -> std::cmp::Ordering { + self.0.id().cmp(&other.0.id()) + } +} + +impl PartialOrd for HandleWrapper { + fn partial_cmp(&self, other: &Self) -> Option { + self.0.id().partial_cmp(&other.0.id()) + } +} + +impl fmt::Debug for HandleWrapper { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + self.0.fmt(f) + } +} + +impl From> for HandleWrapper { + fn from(handle: Handle) -> Self { + Self(handle) + } +} + +unsafe impl Send for HandleWrapper {} +unsafe impl Sync for HandleWrapper {} diff --git a/crates/fj-kernel/src/stores/mod.rs b/crates/fj-kernel/src/stores/mod.rs index 2e9b6a529..3d2c6b257 100644 --- a/crates/fj-kernel/src/stores/mod.rs +++ b/crates/fj-kernel/src/stores/mod.rs @@ -1,10 +1,15 @@ //! Append-only object storage +mod blocks; +mod handle; mod store; use crate::objects::GlobalCurve; -pub use self::store::{Handle, Iter, Reservation, Store}; +pub use self::{ + handle::{Handle, HandleWrapper, ObjectId}, + store::{Iter, Reservation, Store}, +}; /// The available object stores #[derive(Debug, Default)] diff --git a/crates/fj-kernel/src/stores/store.rs b/crates/fj-kernel/src/stores/store.rs index 851637ea5..27d94e53b 100644 --- a/crates/fj-kernel/src/stores/store.rs +++ b/crates/fj-kernel/src/stores/store.rs @@ -21,13 +21,12 @@ //! //! But in any case, this was fun to write, and not that much work. -use std::{ - any::type_name, fmt, hash::Hash, iter, marker::PhantomData, ops::Deref, - sync::Arc, -}; +use std::{marker::PhantomData, sync::Arc}; use parking_lot::RwLock; +use super::{blocks::Blocks, Handle}; + /// Append-only object storage #[derive(Debug)] pub struct Store { @@ -97,152 +96,6 @@ impl<'a, T> IntoIterator for &'a Store { } } -/// A handle for an object -/// -/// You can get an instance of `Handle` by inserting an object into a store. See -/// [`Store::insert`]. A handle dereferences to the object it points to, via its -/// [`Deref`] implementation. -/// -/// # Equality and Identity -/// -/// Equality of `Handle`s is defined via the objects they reference. If those -/// objects are equal, the `Handle`s are considered equal. -/// -/// This is distinct from the *identity* of the referenced objects. Two objects -/// might be equal, but they might be have been created at different times, for -/// different reasons, and thus live in different slots in the storage. This is -/// a relevant distinction when validating objects, as equal but not identical -/// objects might be a sign of a bug. -/// -/// You can compare the identity of two objects through their `Handle`s, by -/// comparing the values returned by [`Handle::id`]. -pub struct Handle { - store: StoreInner, - ptr: *const Option, -} - -impl Handle { - /// Access this pointer's unique id - pub fn id(&self) -> u64 { - self.ptr as u64 - } - - /// Return a clone of the object this handle refers to - pub fn clone_object(&self) -> T - where - T: Clone, - { - self.deref().clone() - } -} - -impl Deref for Handle { - type Target = T; - - fn deref(&self) -> &Self::Target { - // `Handle` keeps a reference to `StoreInner`. Since that is an `Arc` - // under the hood, we know that as long as an instance of `Handle` - // exists, the `StoreInner` its data lives in is still alive. Even if - // the `Store` was dropped. - // - // The `Store` API ensures two things: - // - // 1. That no `Handle` is ever created, until the object it references - // has at least been reserved. - // 2. That the memory objects live in is never deallocated. - // - // That means that as long as a `Handle` exists, the object is - // references has at least been reserved, and has not been deallocated. - // - // Given all this, we know that the following must be true: - // - // - The pointer is not null. - // - The pointer is properly aligned. - // - The pointer is dereferenceable. - // - The pointer points to an initialized instance of `T`. - // - // Further, there is no way to (safely) get a `&mut` reference to any - // object in a `Store`/`Block`. So we know that the aliasing rules for - // the reference we return here are enforced. - // - // Furthermore, all of the code mentioned here is covered by unit tests, - // which I've run successfully run under Miri. - let cell = unsafe { &*self.ptr }; - - // Can only happen, if the object has been reserved, but the reservation - // was never completed. - cell.as_ref() - .expect("Handle references non-existing object") - } -} - -impl Clone for Handle { - fn clone(&self) -> Self { - Self { - store: self.store.clone(), - ptr: self.ptr, - } - } -} - -impl Eq for Handle where T: Eq {} - -impl PartialEq for Handle -where - T: PartialEq, -{ - fn eq(&self, other: &Self) -> bool { - self.deref().eq(other.deref()) - } -} - -impl Hash for Handle -where - T: Hash, -{ - fn hash(&self, state: &mut H) { - self.deref().hash(state) - } -} - -impl Ord for Handle -where - T: Ord, -{ - fn cmp(&self, other: &Self) -> std::cmp::Ordering { - self.deref().cmp(other.deref()) - } -} - -impl PartialOrd for Handle -where - T: PartialOrd, -{ - fn partial_cmp(&self, other: &Self) -> Option { - self.deref().partial_cmp(other.deref()) - } -} - -impl fmt::Debug for Handle { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - let name = { - let type_name = type_name::(); - match type_name.rsplit_once("::") { - Some((_, name)) => name, - None => type_name, - } - }; - let id = self.id(); - - write!(f, "{name} @ {id:#x}")?; - - Ok(()) - } -} - -unsafe impl Send for Handle {} -unsafe impl Sync for Handle {} - /// An iterator over objects in a [`Store`] pub struct Iter<'a, T> { store: StoreInner, @@ -310,131 +163,11 @@ impl Reservation { } } -type StoreInner = Arc>>; - -#[derive(Debug)] -pub struct Blocks { - inner: Vec>, - block_size: usize, -} - -impl Blocks { - pub fn new(block_size: usize) -> Self { - Self { - inner: Vec::new(), - block_size, - } - } - - pub fn push(&mut self, object: T) -> *const Option { - let (index, _) = self.reserve(); - self.insert(index, object) - } - - pub fn reserve(&mut self) -> ((usize, usize), *mut Option) { - let mut current_block = match self.inner.pop() { - Some(block) => block, - None => Block::new(self.block_size), - }; - - let ret = loop { - match current_block.reserve() { - Ok((object_index, ptr)) => { - let block_index = self.inner.len(); - break ((block_index, object_index), ptr); - } - Err(()) => { - // Block is full. Need to create a new one and retry. - self.inner.push(current_block); - current_block = Block::new(self.block_size); - } - } - }; - - self.inner.push(current_block); - - ret - } - - pub fn insert( - &mut self, - (block_index, object_index): (usize, usize), - object: T, - ) -> *const Option { - let block = &mut self.inner[block_index]; - block.insert(object_index, object) - } - - pub fn get(&self, index: usize) -> Option<&Block> { - self.inner.get(index) - } - - #[cfg(test)] - pub fn iter(&self) -> impl Iterator + '_ { - self.inner.iter().flat_map(|block| block.iter()) - } -} - -#[derive(Debug)] -pub struct Block { - objects: Box<[Option]>, - next: usize, -} - -impl Block { - pub fn new(size: usize) -> Self { - let vec = iter::repeat_with(|| None) - .take(size) - .collect::>>(); - let objects = vec.into_boxed_slice(); - - Self { objects, next: 0 } - } - - pub fn reserve(&mut self) -> Result<(usize, *mut Option), ()> { - if self.next >= self.objects.len() { - return Err(()); - } - - let index = self.next; - let ptr = &mut self.objects[self.next]; - self.next += 1; - - Ok((index, ptr)) - } - - pub fn insert(&mut self, index: usize, object: T) -> *const Option { - self.objects[index] = Some(object); - &self.objects[index] - } - - pub fn get(&self, index: usize) -> &Option { - &self.objects[index] - } - - pub fn len(&self) -> usize { - self.next - } - - #[cfg(test)] - pub fn iter(&self) -> impl Iterator + '_ { - let mut i = 0; - iter::from_fn(move || { - if i >= self.len() { - return None; - } - - let object = self.get(i).as_ref()?; - i += 1; - - Some(object) - }) - } -} +pub type StoreInner = Arc>>; #[cfg(test)] mod tests { - use super::{Blocks, Store}; + use super::Store; #[test] fn insert_and_handle() { @@ -477,15 +210,4 @@ mod tests { let objects = store.iter().collect::>(); assert_eq!(objects, [a, b]); } - - #[test] - fn blocks_push() { - let mut blocks = Blocks::new(1); - - blocks.push(0); - blocks.push(1); - - let objects = blocks.iter().copied().collect::>(); - assert_eq!(objects, [0, 1]); - } }