From 079fec3be67f579d1c2134c2d54c59ef6ac0d9d8 Mon Sep 17 00:00:00 2001 From: Jonathan Kelley Date: Mon, 22 Jan 2024 21:17:02 -0800 Subject: [PATCH] simplify generational box by compressing debug info --- packages/generational-box/src/lib.rs | 151 ++------------------ packages/generational-box/src/references.rs | 2 + packages/generational-box/src/sync.rs | 10 +- packages/generational-box/src/unsync.rs | 17 +-- packages/generational-box/tests/basic.rs | 133 +++++++++++++++++ packages/signals/src/lib.rs | 4 +- packages/signals/src/map.rs | 8 +- 7 files changed, 162 insertions(+), 163 deletions(-) create mode 100644 packages/generational-box/tests/basic.rs diff --git a/packages/generational-box/src/lib.rs b/packages/generational-box/src/lib.rs index 058efd158..6092f7b1c 100644 --- a/packages/generational-box/src/lib.rs +++ b/packages/generational-box/src/lib.rs @@ -20,138 +20,6 @@ mod references; mod sync; mod unsync; -/// # Example -/// -/// ```compile_fail -/// let data = String::from("hello world"); -/// let owner = UnsyncStorage::owner(); -/// let key = owner.insert(&data); -/// drop(data); -/// assert_eq!(*key.read(), "hello world"); -/// ``` -#[allow(unused)] -fn compile_fail() {} - -#[test] -fn reused() { - let first_ptr; - { - let owner = UnsyncStorage::owner(); - first_ptr = owner.insert(1).raw.0.data.data_ptr(); - drop(owner); - } - { - let owner = UnsyncStorage::owner(); - let second_ptr = owner.insert(1234).raw.0.data.data_ptr(); - assert_eq!(first_ptr, second_ptr); - drop(owner); - } -} - -#[test] -fn leaking_is_ok() { - let data = String::from("hello world"); - let key; - { - // create an owner - let owner = UnsyncStorage::owner(); - // insert data into the store - key = owner.insert(data); - // don't drop the owner - std::mem::forget(owner); - } - assert_eq!( - key.try_read().as_deref().unwrap(), - &"hello world".to_string() - ); -} - -#[test] -fn drops() { - let data = String::from("hello world"); - let key; - { - // create an owner - let owner = UnsyncStorage::owner(); - // insert data into the store - key = owner.insert(data); - // drop the owner - } - assert!(key.try_read().is_err()); -} - -#[test] -fn works() { - let owner = UnsyncStorage::owner(); - let key = owner.insert(1); - - assert_eq!(*key.read(), 1); -} - -#[test] -fn insert_while_reading() { - let owner = UnsyncStorage::owner(); - let key; - { - let data: String = "hello world".to_string(); - key = owner.insert(data); - } - let value = key.read(); - owner.insert(&1); - assert_eq!(*value, "hello world"); -} - -#[test] -#[should_panic] -fn panics() { - let owner = UnsyncStorage::owner(); - let key = owner.insert(1); - drop(owner); - - assert_eq!(*key.read(), 1); -} - -#[test] -fn fuzz() { - fn maybe_owner_scope( - valid_keys: &mut Vec>, - invalid_keys: &mut Vec>, - path: &mut Vec, - ) { - let branch_cutoff = 5; - let children = if path.len() < branch_cutoff { - rand::random::() % 4 - } else { - rand::random::() % 2 - }; - - for i in 0..children { - let owner = UnsyncStorage::owner(); - let key = owner.insert(format!("hello world {path:?}")); - valid_keys.push(key); - path.push(i); - // read all keys - println!("{:?}", path); - for key in valid_keys.iter() { - let value = key.read(); - println!("{:?}", &*value); - assert!(value.starts_with("hello world")); - } - #[cfg(any(debug_assertions, feature = "check_generation"))] - for key in invalid_keys.iter() { - assert!(!key.validate()); - } - maybe_owner_scope(valid_keys, invalid_keys, path); - invalid_keys.push(valid_keys.pop().unwrap()); - path.pop(); - } - } - - for _ in 0..10 { - maybe_owner_scope(&mut Vec::new(), &mut Vec::new(), &mut Vec::new()); - } -} - /// The type erased id of a generational box. #[derive(Clone, Copy, PartialEq, Eq, Hash)] pub struct GenerationalBoxId { @@ -200,7 +68,7 @@ impl Debug for GenerationalBox { impl> GenerationalBox { #[inline(always)] - fn validate(&self) -> bool { + pub fn validate(&self) -> bool { #[cfg(any(debug_assertions, feature = "check_generation"))] { self.raw @@ -215,6 +83,11 @@ impl> GenerationalBox { } } + /// Get the raw pointer to the value. + pub fn raw_ptr(&self) -> *const () { + self.raw.0.data.data_ptr() + } + /// Get the id of the generational box. pub fn id(&self) -> GenerationalBoxId { GenerationalBoxId { @@ -234,12 +107,11 @@ impl> GenerationalBox { })); } let result = self.raw.0.data.try_read( - #[cfg(any(debug_assertions, feature = "debug_ownership"))] - self.created_at, #[cfg(any(debug_assertions, feature = "debug_ownership"))] GenerationalRefBorrowInfo { borrowed_at: std::panic::Location::caller(), borrowed_from: &self.raw.0.borrow, + created_at: self.created_at, }, ); @@ -272,11 +144,10 @@ impl> GenerationalBox { })); } let result = self.raw.0.data.try_write( - #[cfg(any(debug_assertions, feature = "debug_ownership"))] - self.created_at, #[cfg(any(debug_assertions, feature = "debug_ownership"))] GenerationalRefMutBorrowInfo { borrowed_from: &self.raw.0.borrow, + created_at: self.created_at, }, ); @@ -360,16 +231,12 @@ pub trait Storage: AnyStorage + 'static { /// Try to read the value. Returns None if the value is no longer valid. fn try_read( &'static self, - #[cfg(any(debug_assertions, feature = "debug_ownership"))] - created_at: &'static std::panic::Location<'static>, #[cfg(any(debug_assertions, feature = "debug_ownership"))] at: GenerationalRefBorrowInfo, ) -> Result, BorrowError>; /// Try to write the value. Returns None if the value is no longer valid. fn try_write( &'static self, - #[cfg(any(debug_assertions, feature = "debug_ownership"))] - created_at: &'static std::panic::Location<'static>, #[cfg(any(debug_assertions, feature = "debug_ownership"))] at: GenerationalRefMutBorrowInfo, ) -> Result, BorrowMutError>; @@ -444,8 +311,10 @@ impl MemoryLocationBorrowInfo { struct MemoryLocationInner { data: S, + #[cfg(any(debug_assertions, feature = "check_generation"))] generation: AtomicU32, + #[cfg(any(debug_assertions, feature = "debug_borrows"))] borrow: MemoryLocationBorrowInfo, } diff --git a/packages/generational-box/src/references.rs b/packages/generational-box/src/references.rs index dc079048c..57e828242 100644 --- a/packages/generational-box/src/references.rs +++ b/packages/generational-box/src/references.rs @@ -48,6 +48,7 @@ impl> Deref for GenerationalRef { pub struct GenerationalRefBorrowInfo { pub(crate) borrowed_at: &'static std::panic::Location<'static>, pub(crate) borrowed_from: &'static crate::MemoryLocationBorrowInfo, + pub(crate) created_at: &'static std::panic::Location<'static>, } #[cfg(any(debug_assertions, feature = "debug_borrows"))] @@ -100,6 +101,7 @@ impl> DerefMut for GenerationalRefM pub struct GenerationalRefMutBorrowInfo { /// The location where the borrow occurred. pub(crate) borrowed_from: &'static crate::MemoryLocationBorrowInfo, + pub(crate) created_at: &'static std::panic::Location<'static>, } #[cfg(any(debug_assertions, feature = "debug_borrows"))] diff --git a/packages/generational-box/src/sync.rs b/packages/generational-box/src/sync.rs index 622c4a612..7a611208b 100644 --- a/packages/generational-box/src/sync.rs +++ b/packages/generational-box/src/sync.rs @@ -56,8 +56,6 @@ impl Storage for SyncStorage { fn try_read( &'static self, #[cfg(any(debug_assertions, feature = "debug_ownership"))] - created_at: &'static std::panic::Location<'static>, - #[cfg(any(debug_assertions, feature = "debug_ownership"))] at: crate::GenerationalRefBorrowInfo, ) -> Result, error::BorrowError> { let read = self.0.try_read(); @@ -74,7 +72,7 @@ impl Storage for SyncStorage { .map_err(|_| { error::BorrowError::Dropped(ValueDroppedError { #[cfg(any(debug_assertions, feature = "debug_ownership"))] - created_at, + created_at: at.created_at, }) }) .map(|guard| { @@ -89,8 +87,6 @@ impl Storage for SyncStorage { fn try_write( &'static self, #[cfg(any(debug_assertions, feature = "debug_ownership"))] - created_at: &'static std::panic::Location<'static>, - #[cfg(any(debug_assertions, feature = "debug_ownership"))] at: crate::GenerationalRefMutBorrowInfo, ) -> Result, error::BorrowMutError> { let write = self.0.try_write(); @@ -107,7 +103,7 @@ impl Storage for SyncStorage { .map_err(|_| { error::BorrowMutError::Dropped(ValueDroppedError { #[cfg(any(debug_assertions, feature = "debug_ownership"))] - created_at, + created_at: at.created_at, }) }) .map(|guard| { @@ -141,6 +137,7 @@ impl Storage for SyncStorage { borrow: GenerationalRefBorrowInfo { borrowed_at: borrow.borrowed_at, borrowed_from: borrow.borrowed_from, + created_at: borrow.created_at, }, }) } @@ -162,6 +159,7 @@ impl Storage for SyncStorage { #[cfg(any(debug_assertions, feature = "debug_borrows"))] borrow: GenerationalRefMutBorrowInfo { borrowed_from: borrow.borrowed_from, + created_at: borrow.created_at, }, }) } diff --git a/packages/generational-box/src/unsync.rs b/packages/generational-box/src/unsync.rs index 095487348..e4434bbf9 100644 --- a/packages/generational-box/src/unsync.rs +++ b/packages/generational-box/src/unsync.rs @@ -6,22 +6,16 @@ use crate::{ use std::cell::{Ref, RefCell, RefMut}; /// A unsync storage. This is the default storage type. +#[derive(Default)] pub struct UnsyncStorage(RefCell>>); -impl Default for UnsyncStorage { - fn default() -> Self { - Self(RefCell::new(None)) - } -} - impl Storage for UnsyncStorage { type Ref = GenerationalRef>; type Mut = GenerationalRefMut>; fn try_read( &'static self, - #[cfg(any(debug_assertions, feature = "debug_ownership"))] - created_at: &'static std::panic::Location<'static>, + #[cfg(any(debug_assertions, feature = "debug_ownership"))] at: crate::GenerationalRefBorrowInfo, ) -> Result, error::BorrowError> { @@ -39,7 +33,7 @@ impl Storage for UnsyncStorage { .map_err(|_| { error::BorrowError::Dropped(error::ValueDroppedError { #[cfg(any(debug_assertions, feature = "debug_ownership"))] - created_at, + created_at: at.created_at, }) }) .map(|guard| { @@ -54,8 +48,6 @@ impl Storage for UnsyncStorage { fn try_write( &'static self, #[cfg(any(debug_assertions, feature = "debug_ownership"))] - created_at: &'static std::panic::Location<'static>, - #[cfg(any(debug_assertions, feature = "debug_ownership"))] at: crate::GenerationalRefMutBorrowInfo, ) -> Result, error::BorrowMutError> { let borrow = self.0.try_borrow_mut(); @@ -71,7 +63,7 @@ impl Storage for UnsyncStorage { .map_err(|_| { error::BorrowMutError::Dropped(error::ValueDroppedError { #[cfg(any(debug_assertions, feature = "debug_ownership"))] - created_at, + created_at: at.created_at, }) }) .map(|guard| { @@ -121,6 +113,7 @@ impl Storage for UnsyncStorage { #[cfg(any(debug_assertions, feature = "debug_borrows"))] borrow: GenerationalRefMutBorrowInfo { borrowed_from: borrow.borrowed_from, + created_at: borrow.created_at, }, }) } diff --git a/packages/generational-box/tests/basic.rs b/packages/generational-box/tests/basic.rs new file mode 100644 index 000000000..e250d2369 --- /dev/null +++ b/packages/generational-box/tests/basic.rs @@ -0,0 +1,133 @@ +use generational_box::{AnyStorage, GenerationalBox, UnsyncStorage}; + +/// # Example +/// +/// ```compile_fail +/// let data = String::from("hello world"); +/// let owner = UnsyncStorage::owner(); +/// let key = owner.insert(&data); +/// drop(data); +/// assert_eq!(*key.read(), "hello world"); +/// ``` +#[allow(unused)] +fn compile_fail() {} + +#[test] +fn reused() { + let first_ptr; + { + let owner = UnsyncStorage::owner(); + first_ptr = owner.insert(1).raw_ptr(); + drop(owner); + } + { + let owner = UnsyncStorage::owner(); + let second_ptr = owner.insert(1234).raw_ptr(); + assert_eq!(first_ptr, second_ptr); + drop(owner); + } +} + +#[test] +fn leaking_is_ok() { + let data = String::from("hello world"); + let key; + { + // create an owner + let owner = UnsyncStorage::owner(); + // insert data into the store + key = owner.insert(data); + // don't drop the owner + std::mem::forget(owner); + } + assert_eq!( + key.try_read().as_deref().unwrap(), + &"hello world".to_string() + ); +} + +#[test] +fn drops() { + let data = String::from("hello world"); + let key; + { + // create an owner + let owner = UnsyncStorage::owner(); + // insert data into the store + key = owner.insert(data); + // drop the owner + } + assert!(key.try_read().is_err()); +} + +#[test] +fn works() { + let owner = UnsyncStorage::owner(); + let key = owner.insert(1); + + assert_eq!(*key.read(), 1); +} + +#[test] +fn insert_while_reading() { + let owner = UnsyncStorage::owner(); + let key; + { + let data: String = "hello world".to_string(); + key = owner.insert(data); + } + let value = key.read(); + owner.insert(&1); + assert_eq!(*value, "hello world"); +} + +#[test] +#[should_panic] +fn panics() { + let owner = UnsyncStorage::owner(); + let key = owner.insert(1); + drop(owner); + + assert_eq!(*key.read(), 1); +} + +#[test] +fn fuzz() { + fn maybe_owner_scope( + valid_keys: &mut Vec>, + invalid_keys: &mut Vec>, + path: &mut Vec, + ) { + let branch_cutoff = 5; + let children = if path.len() < branch_cutoff { + rand::random::() % 4 + } else { + rand::random::() % 2 + }; + + for i in 0..children { + let owner = UnsyncStorage::owner(); + let key = owner.insert(format!("hello world {path:?}")); + valid_keys.push(key); + path.push(i); + // read all keys + println!("{:?}", path); + for key in valid_keys.iter() { + let value = key.read(); + println!("{:?}", &*value); + assert!(value.starts_with("hello world")); + } + #[cfg(any(debug_assertions, feature = "check_generation"))] + for key in invalid_keys.iter() { + assert!(!key.validate()); + } + maybe_owner_scope(valid_keys, invalid_keys, path); + invalid_keys.push(valid_keys.pop().unwrap()); + path.pop(); + } + } + + for _ in 0..10 { + maybe_owner_scope(&mut Vec::new(), &mut Vec::new(), &mut Vec::new()); + } +} diff --git a/packages/signals/src/lib.rs b/packages/signals/src/lib.rs index c1117df06..bbe5640e2 100644 --- a/packages/signals/src/lib.rs +++ b/packages/signals/src/lib.rs @@ -22,8 +22,8 @@ pub use dependency::*; mod map; pub use map::*; -mod comparer; -pub use comparer::*; +// mod comparer; +// pub use comparer::*; mod global; pub use global::*; diff --git a/packages/signals/src/map.rs b/packages/signals/src/map.rs index 71379d944..163959c3c 100644 --- a/packages/signals/src/map.rs +++ b/packages/signals/src/map.rs @@ -14,10 +14,14 @@ pub struct MappedSignal { impl MappedSignal<()> { /// Create a new mapped signal. - pub fn new>, U: ?Sized>( + pub fn new( signal: Signal, mapping: impl Fn(&T) -> &U + 'static, - ) -> MappedSignal> { + ) -> MappedSignal> + where + S: Storage>, + U: ?Sized, + { MappedSignal { origin_scope: signal.origin_scope(), mapping: CopyValue::new(Box::new(move || S::map(signal.read(), &mapping))),